[Cluster-devel] cluster/group/daemon app.c

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-06-22 21:12:34

Modified files:
	group/daemon   : app.c 

Log message:
	don't process new join/leave events without quorum

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.32&r2=1.33

--- cluster/group/daemon/app.c	2006/06/22 18:39:26	1.32
+++ cluster/group/daemon/app.c	2006/06/22 21:12:33	1.33
@@ -1373,7 +1373,7 @@
 			log_group(g, "set current event to recovery for %d",
 				  ev->nodeid);
 			list_del(&ev->list);
-		} else if (list_empty(&recovery_sets) &&
+		} else if (list_empty(&recovery_sets) && cman_quorate &&
 			   !list_empty(&a->events)) {
 			ev = list_entry(a->events.next, event_t, list);
 			list_del(&ev->list);

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-07-14 20:45:28

Modified files:
	group/daemon   : app.c 

Log message:
	node A may get a start cb and send a started message, and node B may
	receive the started message before it gets its own start cb; node B
	shouldn't ignore the started message from A.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.34&r2=1.35

--- cluster/group/daemon/app.c	2006/06/28 22:16:36	1.34
+++ cluster/group/daemon/app.c	2006/07/14 20:45:27	1.35
@@ -1079,11 +1079,9 @@
 {
 	node_t *node;
 
-	if (!event_state_starting(a)) {
-		log_error(a->g, "mark_node_started: event not starting %d "
+	if (!event_state_starting(a))
+		log_debug(a->g, "mark_node_started: event not starting %d "
 			  "from %d", a->current_event->state, nodeid);
-		return -1;
-	}
 
 	node = find_app_node(a, nodeid);
 	if (!node) {

[Cluster-devel] cluster/group/daemon app.c

[Fabio Massimo Di Nitto]

teigland at sourceware.org wrote:

> Patches:
> http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.34&r2=1.35
> 
> --- cluster/group/daemon/app.c	2006/06/28 22:16:36	1.34
> +++ cluster/group/daemon/app.c	2006/07/14 20:45:27	1.35
> @@ -1079,11 +1079,9 @@
>  {
>  	node_t *node;
>  
> -	if (!event_state_starting(a)) {
> -		log_error(a->g, "mark_node_started: event not starting %d "
> +	if (!event_state_starting(a))
> +		log_debug(a->g, "mark_node_started: event not starting %d "
>  			  "from %d", a->current_event->state, nodeid);
> -		return -1;
> -	}

Hi,

This commit fails to build. I am not sure if you want log_debug or log_error
there, but in case of log_debug it's enough to remove "a->g,".

Thanks
Fabio

-- 
I'm going to make him an offer he can't refuse.

[Cluster-devel] cluster/group/daemon app.c

[rohara]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	rohara at sourceware.org	2006-07-17 17:06:25

Modified files:
	group/daemon   : app.c 

Log message:
	Remove extra argument from log_debug call.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.35&r2=1.36

--- cluster/group/daemon/app.c	2006/07/14 20:45:27	1.35
+++ cluster/group/daemon/app.c	2006/07/17 17:06:25	1.36
@@ -1080,7 +1080,7 @@
 	node_t *node;
 
 	if (!event_state_starting(a))
-		log_debug(a->g, "mark_node_started: event not starting %d "
+		log_debug("mark_node_started: event not starting %d "
 			  "from %d", a->current_event->state, nodeid);
 
 	node = find_app_node(a, nodeid);

[Cluster-devel] cluster/group/daemon app.c

[Ryan O'Hara]

I've checked-in a fix. I hit this problem this morning as well.

Thanks.

Ryan


Fabio Massimo Di Nitto wrote:
> teigland at sourceware.org wrote:
> 
>> Patches:
>> http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.34&r2=1.35
>>
>> --- cluster/group/daemon/app.c	2006/06/28 22:16:36	1.34
>> +++ cluster/group/daemon/app.c	2006/07/14 20:45:27	1.35
>> @@ -1079,11 +1079,9 @@
>>  {
>>  	node_t *node;
>>  
>> -	if (!event_state_starting(a)) {
>> -		log_error(a->g, "mark_node_started: event not starting %d "
>> +	if (!event_state_starting(a))
>> +		log_debug(a->g, "mark_node_started: event not starting %d "
>>  			  "from %d", a->current_event->state, nodeid);
>> -		return -1;
>> -	}
> 
> Hi,
> 
> This commit fails to build. I am not sure if you want log_debug or log_error
> there, but in case of log_debug it's enough to remove "a->g,".
> 
> Thanks
> Fabio
> 

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-07-17 18:22:57

Modified files:
	group/daemon   : app.c 

Log message:
	fix up debug logging

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.36&r2=1.37

--- cluster/group/daemon/app.c	2006/07/17 17:06:25	1.36
+++ cluster/group/daemon/app.c	2006/07/17 18:22:57	1.37
@@ -1080,7 +1080,7 @@
 	node_t *node;
 
 	if (!event_state_starting(a))
-		log_debug("mark_node_started: event not starting %d "
+		log_group(a->g, "mark_node_started: event not starting %d "
 			  "from %d", a->current_event->state, nodeid);
 
 	node = find_app_node(a, nodeid);

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-07-19 19:45:53

Modified files:
	group/daemon   : app.c 

Log message:
	needed _safe version of list_for_each_entry when moving entries

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.37&r2=1.38

--- cluster/group/daemon/app.c	2006/07/17 18:22:57	1.37
+++ cluster/group/daemon/app.c	2006/07/19 19:45:53	1.38
@@ -1298,7 +1298,7 @@
 	app_t *a = g->app;
 	event_t *ev, *rev;
 	node_t *node;
-	struct nodeid *id;
+	struct nodeid *id, *safe;
 
 	ev = a->current_event;
 	if (!ev)
@@ -1335,7 +1335,7 @@
 		log_group(g, "extend active rev %d with failed node %d",
 			  ev->nodeid, rev->nodeid);
 
-		list_for_each_entry(id, &rev->extended, list) {
+		list_for_each_entry_safe(id, safe, &rev->extended, list) {
 			list_del(&id->list);
 			list_add(&id->list, &ev->extended);
 			log_group(g, "extend active rev %d with failed node %d",

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-08-02 20:30:35

Modified files:
	group/daemon   : app.c 

Log message:
	before freeing a group struct, sanity check it's not referenced in
	any recovery sets

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.38&r2=1.39

--- cluster/group/daemon/app.c	2006/07/19 19:45:53	1.38
+++ cluster/group/daemon/app.c	2006/08/02 20:30:35	1.39
@@ -656,6 +656,8 @@
 
 void finalize_our_leave(group_t *g)
 {
+	struct recovery_set *rs;
+	struct recovery_entry *re, *re2;
 	app_t *a = g->app;
 
 	log_group(g, "finalize_our_leave");
@@ -667,8 +669,19 @@
 	del_app_nodes(a);
 	free(a);
 
-	/* FIXME: check if there are any recovery_sets
-	   referencing this group somehow */
+	/* this group shouldn't be in any recovery sets... sanity check
+	   and avoid future segfault by removing re's referencing this g */
+
+	list_for_each_entry(rs, &recovery_sets, list) {
+		list_for_each_entry_safe(re, re2, &rs->entries, list) {
+			if (re->group == g) {
+				log_error(g, "finalize: still in recovery "
+					  "set %d", rs->nodeid);
+				list_del(&rs->list);
+				free(rs);
+			}
+		}
+	}
 
 	remove_group(g);
 }

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-08-14 21:01:54

Modified files:
	group/daemon   : app.c 

Log message:
	Code that starts groups in order of level during recovery wasn't
	working right in the case where a node fails while mounting, i.e.
	node fails after it's joined the level2 mountgroup but before it
	joins the level1 lockspace.  Code now checks that all lower levels
	are recovered instead of just checking that level-1 is recovered.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.39&r2=1.40

--- cluster/group/daemon/app.c	2006/08/02 20:30:35	1.39
+++ cluster/group/daemon/app.c	2006/08/14 21:01:54	1.40
@@ -379,19 +379,6 @@
 	return 1;
 }
 
-static int level_is_recovered(struct recovery_set *rs, int level)
-{
-	struct recovery_entry *re;
-
-	list_for_each_entry(re, &rs->entries, list) {
-		if (re->group->level != level)
-			continue;
-		if (!re->recovered)
-			return 0;
-	}
-	return 1;
-}
-
 void dump_recovery_sets(void)
 {
 	struct recovery_set *rs;
@@ -407,33 +394,47 @@
 	}
 }
 
-/* lower level group should be recovered in each recovery set */
-
-static int lower_level_recovered(group_t *g)
+static int group_in_recovery_set(struct recovery_set *rs, group_t *g)
 {
-	struct recovery_set *rs;
 	struct recovery_entry *re;
-	int found = 0;
 
 	list_for_each_entry(rs, &recovery_sets, list) {
 		list_for_each_entry(re, &rs->entries, list) {
-			if (re->group == g) {
-				found = 1;
-				if (level_is_recovered(rs, g->level - 1))
-					break;
-				else {
-					log_group(g, "lower level %d is not "
-						  "recovered in rs %d",
-						  g->level - 1, rs->nodeid);
-					/* dump_recovery_sets(); */
-					return 0;
-				}
-			}
+			if (re->group == g)
+				return 1;
 		}
 	}
+	return 0;
+}
+
+static int rs_lower_levels_recovered(struct recovery_set *rs, int level)
+{
+	struct recovery_entry *re;
 
-	if (!found)
+	list_for_each_entry(re, &rs->entries, list) {
+		if (re->group->level < level && !re->recovered)
+			return 0;
+	}
+	return 1;
+}
+
+/* lower level groups should be recovered in each rs this group is in */
+
+static int lower_levels_recovered(group_t *g)
+{
+	struct recovery_set *rs;
+
+	list_for_each_entry(rs, &recovery_sets, list) {
+		if (!group_in_recovery_set(rs, g))
+			continue;
+
+		if (rs_lower_levels_recovered(rs, g->level))
+			continue;
+
+		log_group(g, "lower levels not recovered in rs %d", rs->nodeid);
 		return 0;
+	}
+
 	return 1;
 }
 
@@ -453,24 +454,14 @@
 static int lowest_level(group_t *g)
 {
 	struct recovery_set *rs;
-	struct recovery_entry *re;
-	int found = 0;
 
 	list_for_each_entry(rs, &recovery_sets, list) {
-		list_for_each_entry(re, &rs->entries, list) {
-			if (re->group == g) {
-				found = 1;
-				if (level_is_lowest(rs, g->level))
-					break;
-				else
-					return 0;
-			}
-
-		}
-	}
-
-	if (!found)
+		if (!group_in_recovery_set(rs, g))
+			continue;
+		if (level_is_lowest(rs, g->level))
+			continue;
 		return 0;
+	}
 	return 1;
 }
 
@@ -1006,11 +997,11 @@
 			} else
 				log_group(g, "wait for all_levels_all_stopped");
 		} else {
-			if (lower_level_recovered(g)) {
+			if (lower_levels_recovered(g)) {
 				ev->state = EST_FAIL_START_WAIT;
 				do_start = 1;
 			} else
-				log_group(g, "wait for lower_level_recovered");
+				log_group(g, "wait for lower_levels_recovered");
 		}
 
 		if (!do_start)

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-08-22 14:36:20

Modified files:
	group/daemon   : app.c 

Log message:
	When we're in X_BEGIN state, accept "stopped" messages from other
	nodes.  This applies to bz 202635.
	(There may be a better way to address this, e.g. forcing a new
	FAIL_BEGIN event to be processed before processing any messages)

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.40&r2=1.41

--- cluster/group/daemon/app.c	2006/08/14 21:01:54	1.40
+++ cluster/group/daemon/app.c	2006/08/22 14:36:20	1.41
@@ -787,6 +787,15 @@
 }
 #endif
 
+int event_state_begin(app_t *a)
+{
+	if (a->current_event->state == EST_JOIN_BEGIN ||
+	    a->current_event->state == EST_LEAVE_BEGIN ||
+	    a->current_event->state == EST_FAIL_BEGIN)
+		return TRUE;
+	return FALSE;
+}
+
 int event_state_stopping(app_t *a)
 {
 	if (a->current_event->state == EST_JOIN_STOP_WAIT ||
@@ -1058,8 +1067,12 @@
 {
 	node_t *node;
 
-	if (!event_state_stopping(a)) {
-		log_error(a->g, "mark_node_stopped: event not stopping: "
+	/* we might get a stopped message from another node who's going
+	   through X_BEGIN before we get to X_BEGIN ourselves, so we need
+	   to accept their message if we're in X_BEGIN, too */
+
+	if (!event_state_stopping(a) && !event_state_begin(a)) {
+		log_error(a->g, "mark_node_stopped: event not stopping/begin: "
 			  "state %s from %d",
 			  ev_state_str(a->current_event), nodeid);
 		return -1;

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-08-31 18:20:51

Modified files:
	group/daemon   : app.c 

Log message:
	when we set a recovery event back to the FAIL_BEGIN state, make
	sure that we process the event once before processing any new
	messages.  this is probably a better fix for bz 202635 than I
	added previously where we accept messages more liberally i.e. in
	X_BEGIN states.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.41&r2=1.42

--- cluster/group/daemon/app.c	2006/08/22 14:36:20	1.41
+++ cluster/group/daemon/app.c	2006/08/31 18:20:51	1.42
@@ -1316,6 +1316,7 @@
 	event_t *ev, *rev;
 	node_t *node;
 	struct nodeid *id, *safe;
+	int rv = 0;
 
 	ev = a->current_event;
 	if (!ev)
@@ -1361,7 +1362,7 @@
 
 		list_del(&rev->list);
 		free_event(rev);
-		return 0;
+		return 1;
 	}
 
 	/* Before starting the rev we need to apply the node addition/removal
@@ -1383,7 +1384,7 @@
 		list_del(&rev->list);
 		a->current_event = rev;
 		free_event(ev);
-
+		rv = 1;
 	} else if (event_state_stopping(a)) {
 
 		/* We'll come back through here multiple times until all the
@@ -1397,9 +1398,7 @@
 		mark_node_stopped(a, rev->nodeid);
 		list_for_each_entry(id, &rev->extended, list)
 			mark_node_stopped(a, id->nodeid);
-
-		process_current_event(g);
-
+		rv = 1;
 	} else {
 		log_group(g, "rev for %d delayed for ev %d %s",
 			  rev->nodeid, ev->nodeid, ev_state_str(ev));
@@ -1411,7 +1410,7 @@
 	/* FIXME: if the current event is a leave and the leaving node has
 	   failed, then replace the current event with the rev */
 
-	return 0;
+	return rv;
 }
 
 static int process_app(group_t *g)
@@ -1428,7 +1427,18 @@
 			goto out;
 		rv += ret;
 
-		rv += recover_current_event(g);
+		ret = recover_current_event(g);
+		if (ret > 0) {
+			rv += ret;
+
+			/* it's important that we call process_current_event()
+			   when recover_current_event() returns 1 */
+
+			ret = process_current_event(g);
+			if (ret < 0)
+				goto out;
+			rv += ret;
+		}
 	} else {
 		/* We only take on a new non-recovery event if there are
 		   no recovery sets outstanding.  The new event may be

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-09-14 20:56:55

Modified files:
	group/daemon   : app.c 

Log message:
	Fixes a really stupid bug checked in yesterday that causes groupd
	to seg fault due to referencing a pointer that's not been set yet.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.43&r2=1.44

--- cluster/group/daemon/app.c	2006/09/12 21:57:05	1.43
+++ cluster/group/daemon/app.c	2006/09/14 20:56:55	1.44
@@ -723,10 +723,17 @@
 int do_startdone(char *name, int level, int event_nr)
 {
 	group_t *g;
-	event_t *ev = g->app->current_event;
+	event_t *ev;
 	char *state;
 
 	g = find_group_level(name, level);
+	if (!g) {
+		log_print("do_startdone: no group level %d name %s",
+			  level, name);
+		return -1;
+	}
+
+	ev = g->app->current_event;
 
 	state = ev ? ev_state_str(ev) : "no-event";
 

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-09-15 20:07:15

Modified files:
	group/daemon   : app.c 

Log message:
	positive return code from recover_current_event() should just indicate
	that the event should be processed again, and not added to the return
	value of process_app() which causes the whole thing to be called in
	a loop

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.44&r2=1.45

--- cluster/group/daemon/app.c	2006/09/14 20:56:55	1.44
+++ cluster/group/daemon/app.c	2006/09/15 20:07:15	1.45
@@ -1323,7 +1323,8 @@
 		dump_group(g);
 }
 
-/* handle a node failure while processing an event */
+/* handle a node failure while processing an event. returning > 0 means
+   we want process_current_event() to be called for the group */
 
 int recover_current_event(group_t *g)
 {
@@ -1443,17 +1444,13 @@
 		rv += ret;
 
 		ret = recover_current_event(g);
-		if (ret > 0) {
-			rv += ret;
-
-			/* it's important that we call process_current_event()
-			   when recover_current_event() returns 1 */
+		if (ret <= 0)
+			goto out;
 
-			ret = process_current_event(g);
-			if (ret < 0)
-				goto out;
-			rv += ret;
-		}
+		ret = process_current_event(g);
+		if (ret < 0)
+			goto out;
+		rv += ret;
 	} else {
 		/* We only take on a new non-recovery event if there are
 		   no recovery sets outstanding.  The new event may be
@@ -1481,6 +1478,8 @@
 	return rv;
 }
 
+/* process_apps() will be called again immediately if it returns > 0 */
+
 int process_apps(void)
 {
 	group_t *g, *safe;

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-10-12 14:25:59

Modified files:
	group/daemon   : app.c 

Log message:
	Handle the case where we're the second node being added to the group
	and the only other member fails.  We need to go ahead and process
	our join.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.49&r2=1.50

--- cluster/group/daemon/app.c	2006/10/06 16:55:19	1.49
+++ cluster/group/daemon/app.c	2006/10/12 14:25:58	1.50
@@ -1427,7 +1427,7 @@
 {
 	app_t *a = g->app;
 	event_t *ev, *rev;
-	node_t *node;
+	node_t *node, *us;
 	struct save_msg *save;
 	struct nodeid *id, *safe;
 	int rv = 0;
@@ -1505,6 +1505,31 @@
 		log_group(g, "rev %d is for group we're waiting to join",
 			  rev->nodeid);
 
+		/* If the failed node is the only other app member apart
+		   from us in the pending membership list, then we must go
+		   ahead with our own join event, there will be no remote nodes
+		   processing a rev or an ev for this group.  We send a recover
+		   message so other nodes waiting to join after us will purge
+		   their rev on the group. */
+
+		if (a->node_count == 2) {
+			node = find_app_node(a, rev->nodeid);
+			us = find_app_node(a, our_nodeid);
+
+			if (node && us) {
+				log_group(g, "joining group with one other node"
+					  " now dead rev %d", rev->nodeid);
+				a->node_count--;
+				list_del(&node->list);
+				free(node);
+				send_recover(g, rev);
+				del_recovery_set(g, rev, 1);
+				list_del(&rev->list);
+				free_event(rev);
+				return 0;
+			}
+		}
+
 		/* Look for a remote node with stopped of 1, if we find one,
 		   then fall through to the 'else if (event_state_stopping)'
 		   below.  A remote node with stopped of 1 means we've received

[Cluster-devel] cluster/group/daemon app.c

[teigland]

CVSROOT:	/cvs/cluster
Module name:	cluster
Changes by:	teigland at sourceware.org	2006-10-16 15:52:05

Modified files:
	group/daemon   : app.c 

Log message:
	typo, deleting "rs" instead of "re" when cleaning stuff up

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/cluster/group/daemon/app.c.diff?cvsroot=cluster&r1=1.51&r2=1.52

--- cluster/group/daemon/app.c	2006/10/13 20:07:45	1.51
+++ cluster/group/daemon/app.c	2006/10/16 15:52:05	1.52
@@ -706,8 +706,8 @@
 			if (re->group == g) {
 				log_error(g, "finalize: still in recovery "
 					  "set %d", rs->nodeid);
-				list_del(&rs->list);
-				free(rs);
+				list_del(&re->list);
+				free(re);
 			}
 		}
 	}