Date: Sun, 03 Oct 2010 12:06:53 +0300 From: Mikolaj Golub <to.my.trociny@gmail.com> To: freebsd-fs@freebsd.org Cc: pjd@freebsd.org Subject: Re: hastd: assertion (res->hr_event != NULL) fails in secondary on split-brain Message-ID: <86ocbbir0y.fsf@kopusha.home.net> In-Reply-To: <86aamw4l42.fsf@kopusha.home.net> (Mikolaj Golub's message of "Sat, 02 Oct 2010 19:26:05 %2B0300") References: <86hbh44wgl.fsf@kopusha.home.net> <86aamw4l42.fsf@kopusha.home.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
On Sat, 02 Oct 2010 19:26:05 +0300 Mikolaj Golub wrote to Mikolaj Golub:
MG> What do you think about the attached patch?
Here is updated version of the patch :-)
1) I didn't notice previously that in the different parts kill() was called
with different signals. So in the new version I have child_kill(res, sig)
instead of child_kill(res);
2) Testing hastd I faced another issue (which is not related to my patch, and
shows up also with unpatched hastd. It is easy to hang hastd running the
following test:
for i in `jot 1000`; do
hastctl role primary storage
hastctl role secondary storage
done
Other host should be configured as secondary and should have some fix for the
problem described initially in this thread (avoid double close of
res->hr_event) so secondary would not crash on assertion.
hastd hangs when switching to secondary, killing primary worker and waiting
for it:
root 1631 0.0 0.5 11244 2364 ?? Is 12:18PM 0:00.37 /sbin/hastd -ddd
root 1869 0.0 7.0 49004 35700 ?? I 12:37PM 0:01.42 hastd: storage (primary) (hastd)
root 1937 0.0 0.3 10924 1764 ?? I 12:37PM 0:00.02 /sbin/hastctl role secondary storage
lolek# gdb /usr/obj/usr/src/sbin/hastd/hastd 1631
[Switching to Thread 28404140 (LWP 100045)]
0x282ba689 in wait4 () from /lib/libc.so.7
(gdb) bt
#0 0x282ba689 in wait4 () from /lib/libc.so.7
#1 0x282912a3 in waitpid () from /lib/libc.so.7
#2 0x280df272 in waitpid () from /lib/libthr.so.3
#3 0x0804c6d4 in control_set_role_common (cfg=0x28419600, nvout=0x2850e0d0, role=3 '\003',
res=0x284eb500, name=0x284a3442 "storage", no=0) at /usr/src/sbin/hastd/control.c:115
#4 0x0804d001 in control_handle (cfg=0x28419600) at /usr/src/sbin/hastd/control.c:356
#5 0x08050555 in main_loop () at /usr/src/sbin/hastd/hastd.c:671
#6 0x08050a7f in main (argc=0, argv=0xbfbfed04) at /usr/src/sbin/hastd/hastd.c:784
(gdb) fr 3
#3 0x0804c6d4 in control_set_role_common (cfg=0x28419600, nvout=0x2850e0d0, role=3 '\003',
res=0x284eb500, name=0x284a3442 "storage", no=0) at /usr/src/sbin/hastd/control.c:115
115 } else if (waitpid(res->hr_workerpid, NULL, 0) !=
(gdb) list
110 if (res->hr_workerpid != 0) {
111 if (kill(res->hr_workerpid, SIGTERM) < 0) {
112 pjdlog_errno(LOG_WARNING,
113 "Unable to kill worker process %u",
114 (unsigned int)res->hr_workerpid);
115 } else if (waitpid(res->hr_workerpid, NULL, 0) !=
116 res->hr_workerpid) {
117 pjdlog_errno(LOG_WARNING,
118 "Error while waiting for worker process %u",
119 (unsigned int)res->hr_workerpid);
It looks like the worker does not die because guard_thread() is sending
CONNECT event to parent and waiting for a response.
lolek# gdb /usr/obj/usr/src/sbin/hastd/hastd 1869
Thread 8 (Thread 28404140 (LWP 100075)):
#0 0x28301ed7 in recvfrom () from /lib/libc.so.7
#1 0x28287f52 in recv () from /lib/libc.so.7
#2 0x0805f467 in proto_common_recv (fd=10, data=0xbfbfe967 "(", size=5)
at /usr/src/sbin/hastd/proto_common.c:77
#3 0x0805f8bd in sp_recv (ctx=0x2850e110, data=0xbfbfe967 "(", size=5)
at /usr/src/sbin/hastd/proto_socketpair.c:185
#4 0x0805ee91 in proto_recv (conn=0x2850e100, data=0xbfbfe967, size=5)
at /usr/src/sbin/hastd/proto.c:207
#5 0x0804e4ae in hast_proto_recv_hdr (conn=0x2850e100, nvp=0xbfbfe9a4)
at /usr/src/sbin/hastd/hast_proto.c:308
#6 0x0804dbbc in event_send (res=0x284eb500, event=1) at /usr/src/sbin/hastd/event.c:69
#7 0x0805a411 in init_remote (res=0x284eb500, inp=0xbfbfea34, outp=0xbfbfea30)
at /usr/src/sbin/hastd/primary.c:661
#8 0x0805e48a in guard_one (res=0x284eb500, ncomp=1) at /usr/src/sbin/hastd/primary.c:1912
#9 0x0805e7e9 in guard_thread (arg=0x284eb500) at /usr/src/sbin/hastd/primary.c:1977
#10 0x0805ac5f in hastd_primary (res=0x284eb500) at /usr/src/sbin/hastd/primary.c:823
#11 0x0804c743 in control_set_role_common (cfg=0x28419600, nvout=0x2850e0d0, role=2 '\002',
res=0x284eb500, name=0x284a3442 "storage", no=0) at /usr/src/sbin/hastd/control.c:129
#12 0x0804d001 in control_handle (cfg=0x28419600) at /usr/src/sbin/hastd/control.c:356
#13 0x08050555 in main_loop () at /usr/src/sbin/hastd/hastd.c:671
#14 0x08050a7f in main (argc=0, argv=0xbfbfed04) at /usr/src/sbin/hastd/hastd.c:784
After changing the code so parent closes hr_event and hr_ctrl after kill()
but before wait() the hang has not been observed.
--
Mikolaj Golub
--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=hast.child_kill.patch
Index: sbin/hastd/hastd.c
===================================================================
--- sbin/hastd/hastd.c (revision 213377)
+++ sbin/hastd/hastd.c (working copy)
@@ -94,22 +94,6 @@ g_gate_load(void)
}
static void
-child_exit_log(unsigned int pid, int status)
-{
-
- if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
- pjdlog_debug(1, "Worker process exited gracefully (pid=%u).",
- pid);
- } else if (WIFSIGNALED(status)) {
- pjdlog_error("Worker process killed (pid=%u, signal=%d).",
- pid, WTERMSIG(status));
- } else {
- pjdlog_error("Worker process exited ungracefully (pid=%u, exitcode=%d).",
- pid, WIFEXITED(status) ? WEXITSTATUS(status) : -1);
- }
-}
-
-static void
child_exit(void)
{
struct hast_resource *res;
@@ -388,8 +372,6 @@ listen_accept(void)
const unsigned char *token;
char laddr[256], raddr[256];
size_t size;
- pid_t pid;
- int status;
proto_local_address(cfg->hc_listenconn, laddr, sizeof(laddr));
pjdlog_debug(1, "Accepting connection to %s.", laddr);
@@ -504,26 +486,7 @@ listen_accept(void)
"Worker process exists (pid=%u), stopping it.",
(unsigned int)res->hr_workerpid);
/* Stop child process. */
- if (kill(res->hr_workerpid, SIGINT) < 0) {
- pjdlog_errno(LOG_ERR,
- "Unable to stop worker process (pid=%u)",
- (unsigned int)res->hr_workerpid);
- /*
- * Other than logging the problem we
- * ignore it - nothing smart to do.
- */
- }
- /* Wait for it to exit. */
- else if ((pid = waitpid(res->hr_workerpid,
- &status, 0)) != res->hr_workerpid) {
- /* We can only log the problem. */
- pjdlog_errno(LOG_ERR,
- "Waiting for worker process (pid=%u) failed",
- (unsigned int)res->hr_workerpid);
- } else {
- child_exit_log(res->hr_workerpid, status);
- }
- child_cleanup(res);
+ child_kill(res, SIGINT);
} else if (res->hr_remotein != NULL) {
char oaddr[256];
@@ -678,8 +641,8 @@ main_loop(void)
if (event_recv(res) == 0)
continue;
/* The worker process exited? */
- proto_close(res->hr_event);
- res->hr_event = NULL;
+ if (res->hr_workerpid != 0)
+ child_kill(res, SIGINT);
}
}
}
Index: sbin/hastd/control.c
===================================================================
--- sbin/hastd/control.c (revision 213377)
+++ sbin/hastd/control.c (working copy)
@@ -63,6 +63,52 @@ child_cleanup(struct hast_resource *res)
res->hr_workerpid = 0;
}
+void
+child_exit_log(unsigned int pid, int status)
+{
+
+ if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
+ pjdlog_debug(1, "Worker process exited gracefully (pid=%u).",
+ pid);
+ } else if (WIFSIGNALED(status)) {
+ pjdlog_error("Worker process killed (pid=%u, signal=%d).",
+ pid, WTERMSIG(status));
+ } else {
+ pjdlog_error("Worker process exited ungracefully (pid=%u, exitcode=%d).",
+ pid, WIFEXITED(status) ? WEXITSTATUS(status) : -1);
+ }
+}
+
+void
+child_kill(struct hast_resource *res, int sig)
+{
+ pid_t pid;
+ int status;
+
+ assert(res->hr_workerpid != 0);
+
+ pid = res->hr_workerpid;
+
+ if (kill(pid, sig) < 0) {
+ pjdlog_errno(LOG_ERR,
+ "Unable to stop worker process (pid=%u)",
+ (unsigned int)pid);
+ /*
+ * Other than logging the problem we
+ * ignore it - nothing smart to do.
+ */
+ }
+ child_cleanup(res);
+ /* Wait for it to exit. */
+ if (waitpid(pid, &status, 0) != pid) {
+ /* We can only log the problem. */
+ pjdlog_errno(LOG_ERR,
+ "Waiting for worker process (pid=%u) failed",
+ (unsigned int)pid);
+ }
+ child_exit_log(pid, status);
+}
+
static void
control_set_role_common(struct hastd_config *cfg, struct nv *nvout,
uint8_t role, struct hast_resource *res, const char *name, unsigned int no)
@@ -107,22 +153,8 @@ control_set_role_common(struct hastd_config *cfg,
* If previous role was primary or secondary we have to kill process
* doing that work.
*/
- if (res->hr_workerpid != 0) {
- if (kill(res->hr_workerpid, SIGTERM) < 0) {
- pjdlog_errno(LOG_WARNING,
- "Unable to kill worker process %u",
- (unsigned int)res->hr_workerpid);
- } else if (waitpid(res->hr_workerpid, NULL, 0) !=
- res->hr_workerpid) {
- pjdlog_errno(LOG_WARNING,
- "Error while waiting for worker process %u",
- (unsigned int)res->hr_workerpid);
- } else {
- pjdlog_debug(1, "Worker process %u stopped.",
- (unsigned int)res->hr_workerpid);
- }
- child_cleanup(res);
- }
+ if (res->hr_workerpid != 0)
+ child_kill(res, SIGTERM);
/* Start worker process if we are changing to primary. */
if (role == HAST_ROLE_PRIMARY)
Index: sbin/hastd/control.h
===================================================================
--- sbin/hastd/control.h (revision 213377)
+++ sbin/hastd/control.h (working copy)
@@ -39,6 +39,8 @@ struct hastd_config;
struct hast_resource;
void child_cleanup(struct hast_resource *res);
+void child_exit_log(unsigned int pid, int status);
+void child_kill(struct hast_resource *res, int sig);
void control_set_role(struct hast_resource *res, uint8_t role);
--=-=-=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86ocbbir0y.fsf>