cluster: STABLE3 - fenced: partition merge handling

Fabio M. Di Nitto fabbione at fedoraproject.org
Mon Oct 5 03:49:40 UTC 2009


Gitweb:        http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=bcc5fdef8473d99399c624a7bc15423a2af645c1
Commit:        bcc5fdef8473d99399c624a7bc15423a2af645c1
Parent:        ff6ab5f989b19c84f026c2e4aaf8704c44387853
Author:        David Teigland <teigland at redhat.com>
AuthorDate:    Tue Sep 15 13:00:57 2009 -0500
Committer:     David Teigland <teigland at redhat.com>
CommitterDate: Tue Sep 15 13:52:24 2009 -0500

fenced: partition merge handling

Remove the need for cman to have dirty state and detect/flag merged
nodes as "disallowed".

The cluster3 fenced has always handled partition merging in part,
rejecting start messages from merged nodes using started_count.
This prevented most harm from merging, but left things in strange
states that needed to be manually resolved.  Now, fenced sorts
things out itself, mainly by killing merged nodes if one partition
maintained quorum.

. don't skip a fencing victim that merges; distinguish stateful merges
  from new joins based on protocol messages

. kill node (cman) that merges statefully; if we've been quorate since
  it left

. don't fence a second time due to the cman kill; master node doing
  fencing needs to remove victims in response to its victim_done
  message like other nodes

. code that matches config changes needed to be changed because it
  couldn't distinguish between old and new failures (in the case of
  a partition merge, we see a node fail,join,fail,join)

. add some new and improve some existing debug messages

. consistent naming routines for the two cpg's for daemon and domain

There's a lengthy comment describing one test case that's still a
problem.  After several attempts at fixing, I don't think it's
possible without some additional help from cman/cpg.  The test is:

a. members A,B,C,D
b. partition A / B,C,D
   B begins fencing A, fencing is failing, so B continues retrying
c. merge A,B,C,D
   nodes B,C,D detect the merge and kill cman on node A
d. members B,C,D
e. members A,B,C,D
   node A is cleanly restarted after being killed
   node B sees A is now ok and stops trying to fence it

One problem is on A in step e, where A receives B's start message
for step c and wrongly thinks it refers to step e (steps c and e
are identical membership transitions B,C,D -> A,B,C,D).

The second problem is on B in step e, where B sees A fail in cpg
from step d and waits for cman to report the same failure.  It
doesn't know that B has cleanly rejoined since cman reported this
failure.

Signed-off-by: David Teigland <teigland at redhat.com>
---
 fence/fenced/cpg.c         |  457 ++++++++++++++++++++++++++++++++++++--------
 fence/fenced/fd.h          |   18 ++-
 fence/fenced/group.c       |    6 +-
 fence/fenced/main.c        |   25 ++--
 fence/fenced/member_cman.c |   70 ++++++-
 fence/fenced/recover.c     |   48 ++++--
 6 files changed, 501 insertions(+), 123 deletions(-)

diff --git a/fence/fenced/cpg.c b/fence/fenced/cpg.c
index 2a43705..c4d649d 100644
--- a/fence/fenced/cpg.c
+++ b/fence/fenced/cpg.c
@@ -1,6 +1,8 @@
 #include "fd.h"
 #include "config.h"
 
+#define PV_STATEFUL 0x0001
+
 struct protocol_version {
 	uint16_t major;
 	uint16_t minor;
@@ -22,6 +24,10 @@ struct protocol {
 struct node_daemon {
 	struct list_head list;
 	int nodeid;
+	int is_member;
+	int killed;
+	uint64_t join_time;
+	uint64_t left_time;
 	struct protocol proto;
 };
 
@@ -67,12 +73,65 @@ struct id_info {
 };
 
 static cpg_handle_t cpg_handle_daemon;
-static int daemon_cpg_fd;
+static int cpg_fd_daemon;
 static struct protocol our_protocol;
 static struct list_head daemon_nodes;
 static struct cpg_address daemon_member[MAX_NODES];
 static int daemon_member_count;
 
+static void log_config(const struct cpg_name *group_name,
+		       const struct cpg_address *member_list,
+		       size_t member_list_entries,
+		       const struct cpg_address *left_list,
+		       size_t left_list_entries,
+		       const struct cpg_address *joined_list,
+		       size_t joined_list_entries)
+{
+	char m_buf[128];
+	char j_buf[32];
+	char l_buf[32];
+	size_t i, len, pos;
+	int ret;
+
+	memset(m_buf, 0, sizeof(m_buf));
+	memset(j_buf, 0, sizeof(j_buf));
+	memset(l_buf, 0, sizeof(l_buf));
+
+	len = sizeof(m_buf);
+	pos = 0;
+	for (i = 0; i < member_list_entries; i++) {
+		ret = snprintf(m_buf + pos, len - pos, " %d",
+			       member_list[i].nodeid);
+		if (ret >= len - pos)
+			break;
+		pos += ret;
+	}
+
+	len = sizeof(j_buf);
+	pos = 0;
+	for (i = 0; i < joined_list_entries; i++) {
+		ret = snprintf(j_buf + pos, len - pos, " %d",
+			       joined_list[i].nodeid);
+		if (ret >= len - pos)
+			break;
+		pos += ret;
+	}
+
+	len = sizeof(l_buf);
+	pos = 0;
+	for (i = 0; i < left_list_entries; i++) {
+		ret = snprintf(l_buf + pos, len - pos, " %d",
+			       left_list[i].nodeid);
+		if (ret >= len - pos)
+			break;
+		pos += ret;
+	}
+
+	log_debug("%s conf %zu %zu %zu memb%s join%s left%s", group_name->value,
+		  member_list_entries, joined_list_entries, left_list_entries,
+		  m_buf, j_buf, l_buf);
+}
+
 static void fd_info_in(struct fd_info *fi)
 {
 	fi->fd_info_size  = le32_to_cpu(fi->fd_info_size);
@@ -225,6 +284,17 @@ void free_cg(struct change *cg)
 	free(cg);
 }
 
+static struct node *get_node_victim(struct fd *fd, int nodeid)
+{
+	struct node *node;
+
+	list_for_each_entry(node, &fd->victims, list) {
+		if (node->nodeid == nodeid)
+			return node;
+	}
+	return NULL;
+}
+
 static struct node_history *get_node_history(struct fd *fd, int nodeid)
 {
 	struct node_history *node;
@@ -253,6 +323,41 @@ void node_history_init(struct fd *fd, int nodeid)
 	list_add_tail(&node->list, &fd->node_history);
 }
 
+void node_history_cman_add(int nodeid)
+{
+	struct fd *fd;
+	struct node_history *node;
+
+	list_for_each_entry(fd, &domains, list) {
+		node_history_init(fd, nodeid);
+
+		node = get_node_history(fd, nodeid);
+		if (!node) {
+			log_error("node_history_cman_add no nodeid %d", nodeid);
+			return;
+		}
+
+		node->cman_add_time = time(NULL);
+	}
+}
+
+void node_history_cman_remove(int nodeid)
+{
+	struct fd *fd;
+	struct node_history *node;
+
+	list_for_each_entry(fd, &domains, list) {
+		node = get_node_history(fd, nodeid);
+		if (!node) {
+			log_error("node_history_cman_remove no nodeid %d",
+				  nodeid);
+			return;
+		}
+
+		node->cman_remove_time = time(NULL);
+	}
+}
+
 static void node_history_start(struct fd *fd, int nodeid)
 {
 	struct node_history *node;
@@ -460,7 +565,7 @@ void send_victim_done(struct fd *fd, int victim)
 	hd->type = FD_MSG_VICTIM_DONE;
 	hd->msgdata = cg->seq;
 
-	if (fd->init_complete)
+	if (fd->init_complete || fd->local_init_complete)
 		hd->flags |= FD_MFLG_COMPLETE;
 
 	node = get_node_history(fd, victim);
@@ -483,11 +588,18 @@ void send_victim_done(struct fd *fd, int victim)
 	free(buf);
 }
 
+/* The master needs to remove the victim upon receiving its own victim_done
+   message, just like everyone else.  Otherwise, when a partitioned node is
+   remerged and then killed, the others will see it's already a victim, but
+   the master won't see that confchg until after it's done fencing the
+   victim for the initial partition.  So when the master sees the second
+   failure (due to the kill), the node won't be a victim already so it'll
+   become a victim again. */
+
 static void receive_victim_done(struct fd *fd, struct fd_header *hd, int len)
 {
 	struct node *node;
 	uint32_t seq = hd->msgdata;
-	int found;
 	struct id_info *id;
 
 	log_debug("receive_victim_done %d:%u flags %x len %d", hd->nodeid, seq,
@@ -495,33 +607,34 @@ static void receive_victim_done(struct fd *fd, struct fd_header *hd, int len)
 
 	/* check that hd->nodeids is fd->master ? */
 
-	/* I don't think there's any problem with the master removing the
-	   victim when it's done instead of waiting to remove it when it
-	   receives its own victim_done message, like the other nodes do */
+	id = (struct id_info *)((char *)hd + sizeof(struct fd_header));
+	id_info_in(id);
 
-	if (hd->nodeid == our_nodeid)
+	node = get_node_victim(fd, id->nodeid);
+	if (!node) {
+		log_debug("receive_victim_done %d:%u no victim nodeid %d",
+			  hd->nodeid, seq, id->nodeid);
 		return;
+	}
 
-	id = (struct id_info *)((char *)hd + sizeof(struct fd_header));
-	id_info_in(id);
+	log_debug("receive_victim_done %d:%u remove victim %d how %d",
+		  hd->nodeid, seq, id->nodeid, id->fence_how);
 
-	found = 0;
-	list_for_each_entry(node, &fd->victims, list) {
-		if (node->nodeid == id->nodeid) {
-			log_debug("receive_victim_done remove nodeid %d how %d",
-				  id->nodeid, id->fence_how);
-			node_history_fence(fd, id->nodeid, id->fence_master,
-					   id->fence_how, id->fence_time);
-			list_del(&node->list);
-			free(node);
-			found = 1;
-			break;
-		}
+	if (hd->nodeid == our_nodeid) {
+		/* sanity check, I don't think this should happen;
+		   see comment in fence_victims() */
+		if (!node->local_victim_done)
+			log_error("expect local_victim_done");
+		node->local_victim_done = 0;
+	} else {
+		/* save details of fencing operation from master, which
+		   master saves at the time it completes it */
+		node_history_fence(fd, id->nodeid, id->fence_master,
+				   id->fence_how, id->fence_time);
 	}
 
-	if (!found)
-		log_debug("receive_victim_done no nodeid %d from %d",
-			  id->nodeid, hd->nodeid);
+	list_del(&node->list);
+	free(node);
 }
 
 static int check_quorum_done(struct fd *fd)
@@ -542,7 +655,7 @@ static int check_quorum_done(struct fd *fd)
 		if (!node->check_quorum)
 			continue;
 
-		if (!is_cman_member(node->nodeid)) {
+		if (!is_cman_member_reread(node->nodeid)) {
 			node->check_quorum = 0;
 		} else {
 			log_debug("check_quorum %d is_cman_member",
@@ -659,6 +772,7 @@ static int match_change(struct fd *fd, struct change *cg, struct fd_header *hd,
 {
 	struct id_info *id;
 	struct member *memb;
+	struct node_history *node;
 	uint32_t seq = hd->msgdata;
 	int i, members_mismatch;
 
@@ -682,6 +796,30 @@ static int match_change(struct fd *fd, struct change *cg, struct fd_header *hd,
 		return 0;
 	}
 
+	if (memb->start) {
+		log_debug("match_change %d:%u skip cg %u already start",
+			  hd->nodeid, seq, cg->seq);
+		return 0;
+	}
+
+	/* a node's start can't match a change if the node joined the cluster
+	   more recently than the change was created */
+
+	node = get_node_history(fd, hd->nodeid);
+	if (!node) {
+		log_debug("match_change %d:%u skip cg %u no node history",
+			  hd->nodeid, seq, cg->seq);
+		return 0;
+	}
+
+	if (node->cman_add_time > cg->create_time) {
+		log_debug("match_change %d:%u skip cg %u created %llu "
+			  "cman add %llu", hd->nodeid, seq, cg->seq,
+			  (unsigned long long)cg->create_time,
+			  (unsigned long long)node->cman_add_time);
+		return 0;
+	}
+
 	/* verify this is the right change by matching the counts
 	   and the nodeids of the current members */
 
@@ -739,6 +877,39 @@ static int match_change(struct fd *fd, struct change *cg, struct fd_header *hd,
    In step 4, how do the nodes know whether the start message from C is
    for confchg1 or confchg2?  Hopefully by comparing the counts and members. */
 
+/* fails to handle partition, merge, kill, failing/manual fence, rejoin.
+   a. m=A,B,C,D
+   b. m=A/B,C,D partition, node B fencing node A, manual, or agent failing
+   c. m=A,B,C,D merge, nodes B,C,D kill cman on node A
+   d. m=B,C,D
+   e. m=A,B,C,D clean rejoin by node A
+   f. node B quits fencing retries on node A and sends victim_done/MEMBER/A
+
+   The problem is that node A in step e receives node B's start message
+   from step c and wrongly thinks it's for step e (member and join lists
+   are the same m=A,B,C,D j=A).  Node B sends the start message for step c
+   so late because it was busy retrying fencing until node A cleanly rejoined
+   in step e, then moved on to process cpg events c, d and e.
+
+   Second problem with this test (not related to change matching) is node B
+   gets stuck on cman is_member checks for the failure of node A due to the
+   kill.  It's waiting for B to become a non-member in cman due to the failure,
+   but B has rejoined since that past failure and won't go away again.
+
+   Possible solution to both problmes:
+   Wait for cpg and cman to be in sync with each other (matching ringid's
+   from both api's?) in wait_conditions.
+
+   a) To replace quorum_done check. (it would be ok for cman ringid to
+      be > cpg ringid for this check but not for other).  This ensures that
+      cman is not lagging behind cpg.  Waits for cman to catch up with cpg.
+
+   b) To avoid sending start for an old cpg confchg.  If cman ringid
+      is > cpg ringid, then return 0 for conditions_done so we won't send
+      start and will wait until the most recent cpg confchg (matching the
+      current cman one) to send a start.  Waits for cpg to catch up with cman.
+*/
+
 static struct change *find_change(struct fd *fd, struct fd_header *hd,
 				  struct fd_info *fi, struct id_info *ids)
 {
@@ -799,7 +970,7 @@ static void receive_start(struct fd *fd, struct fd_header *hd, int len)
 
 	added = is_added(fd, hd->nodeid);
 
-	if (added && fi->started_count) {
+	if (added && fi->started_count && fd->started_count) {
 		log_error("receive_start %d:%u add node with started_count %u",
 			  hd->nodeid, seq, fi->started_count);
 
@@ -817,6 +988,12 @@ static void receive_start(struct fd *fd, struct fd_header *hd, int len)
 		   assumes a joining node won't ever see an existing node
 		   as "added" under normal circumstances. */
 
+		/* The fd->started_count condition is needed for the case
+		   where a node begins partitioned, has never started, and then
+		   is merged with other nodes that have started.  We don't
+		   want this unstarted node to reject the started ones even
+		   though it sees them as "added". */
+
 		memb->disallowed = 1;
 		return;
 	}
@@ -911,7 +1088,7 @@ static void send_info(struct fd *fd, int type)
 	hd->msgdata = cg->seq;
 	if (cg->we_joined)
 		hd->flags |= FD_MFLG_JOINING;
-	if (fd->init_complete)
+	if (fd->init_complete || fd->local_init_complete)
 		hd->flags |= FD_MFLG_COMPLETE;
 
 	/* fill in fd_info */
@@ -984,7 +1161,7 @@ static int nodes_added(struct fd *fd)
    victims list.  So, we need to assume that any node in cluster.conf that's
    not a cluster member when we're added to the fd is already a victim.
    We can go back on that assumption, and clear out any presumed victims, when
-   we see a message from a previous member saying that are no current victims. */
+   we see a message from a previous member saying that are no current victims.*/
 
 static void add_victims(struct fd *fd, struct change *cg)
 {
@@ -1000,7 +1177,7 @@ static void add_victims(struct fd *fd, struct change *cg)
 			   disallowed node is killed.  The original
 			   partition makes the node a victim, and killing
 			   it after a merge will find it already a victim. */
-			log_debug("add_victims nodeid %d already victim",
+			log_debug("add_victims node %d already victim",
 				  memb->nodeid);
 			continue;
 		}
@@ -1008,7 +1185,7 @@ static void add_victims(struct fd *fd, struct change *cg)
 		if (!node)
 			return;
 		list_add(&node->list, &fd->victims);
-		log_debug("add nodeid %d to victims", node->nodeid);
+		log_debug("add_victims node %d", node->nodeid);
 	}
 }
 
@@ -1048,6 +1225,7 @@ static void apply_changes(struct fd *fd)
 
 	case CGST_WAIT_MESSAGES:
 		if (wait_messages_done(fd)) {
+			our_protocol.dr_ver.flags |= PV_STATEFUL;
 			set_master(fd);
 			cg->state = CGST_WAIT_FENCING;  /* for queries */
 
@@ -1055,6 +1233,7 @@ static void apply_changes(struct fd *fd)
 				delay_fencing(fd, nodes_added(fd));
 				fence_victims(fd);
 				send_complete(fd);
+				fd->local_init_complete = 1;
 			} else {
 				defer_fencing(fd);
 			}
@@ -1100,6 +1279,7 @@ static int add_change(struct fd *fd,
 	INIT_LIST_HEAD(&cg->removed);
 	cg->seq = ++fd->change_seq;
 	cg->state = CGST_WAIT_CONDITIONS;
+	cg->create_time = time(NULL);
 
 	cg->member_count = member_list_entries;
 	cg->joined_count = joined_list_entries;
@@ -1187,7 +1367,7 @@ static void add_victims_init(struct fd *fd, struct change *cg)
 	list_for_each_entry_safe(node, safe, &fd->complete, list) {
 		list_del(&node->list);
 
-		if (!is_cman_member(node->nodeid) &&
+		if (!is_cman_member_reread(node->nodeid) &&
 		    !find_memb(cg, node->nodeid) &&
 		    !is_victim(fd, node->nodeid)) {
 			node->init_victim = 1;
@@ -1199,7 +1379,8 @@ static void add_victims_init(struct fd *fd, struct change *cg)
 	}
 }
 
-static int we_left(const struct cpg_address *left_list, size_t left_list_entries)
+static int we_left(const struct cpg_address *left_list,
+		   size_t left_list_entries)
 {
 	int i;
 
@@ -1210,19 +1391,23 @@ static int we_left(const struct cpg_address *left_list, size_t left_list_entries
 	return 0;
 }
 
-static void confchg_cb(cpg_handle_t handle,
-		       const struct cpg_name *group_name,
-		       const struct cpg_address *member_list,
-		       size_t member_list_entries,
-		       const struct cpg_address *left_list,
-		       size_t left_list_entries,
-		       const struct cpg_address *joined_list,
-		       size_t joined_list_entries)
+static void confchg_cb_domain(cpg_handle_t handle,
+			      const struct cpg_name *group_name,
+			      const struct cpg_address *member_list,
+			      size_t member_list_entries,
+			      const struct cpg_address *left_list,
+			      size_t left_list_entries,
+			      const struct cpg_address *joined_list,
+			      size_t joined_list_entries)
 {
 	struct fd *fd;
 	struct change *cg;
 	int rv;
 
+	log_config(group_name, member_list, member_list_entries,
+		   left_list, left_list_entries,
+		   joined_list, joined_list_entries);
+
 	fd = find_fd_handle(handle);
 	if (!fd) {
 		log_error("confchg_cb no fence domain for cpg %s",
@@ -1275,10 +1460,10 @@ static void fd_header_in(struct fd_header *hd)
 	hd->msgdata     = le32_to_cpu(hd->msgdata);
 }
 
-static void deliver_cb(cpg_handle_t handle,
-		       const struct cpg_name *group_name,
-		       uint32_t nodeid, uint32_t pid,
-		       void *data, size_t len)
+static void deliver_cb_domain(cpg_handle_t handle,
+			      const struct cpg_name *group_name,
+			      uint32_t nodeid, uint32_t pid,
+			      void *data, size_t len)
 {
 	struct fd *fd;
 	struct fd_header *hd;
@@ -1332,19 +1517,19 @@ static void deliver_cb(cpg_handle_t handle,
 	apply_changes(fd);
 }
 
-static cpg_callbacks_t cpg_callbacks = {
-	.cpg_deliver_fn = deliver_cb,
-	.cpg_confchg_fn = confchg_cb,
+static cpg_callbacks_t cpg_callbacks_domain = {
+	.cpg_deliver_fn = deliver_cb_domain,
+	.cpg_confchg_fn = confchg_cb_domain,
 };
 
-static void process_fd_cpg(int ci)
+static void process_cpg_domain(int ci)
 {
 	struct fd *fd;
 	cpg_error_t error;
 
 	fd = find_fd_ci(ci);
 	if (!fd) {
-		log_error("process_fd_cpg no fence domain for ci %d", ci);
+		log_error("process_cpg_domain no fence domain for ci %d", ci);
 		return;
 	}
 
@@ -1362,7 +1547,7 @@ int fd_join(struct fd *fd)
 	struct cpg_name name;
 	int i = 0, f, ci;
 
-	error = cpg_initialize(&h, &cpg_callbacks);
+	error = cpg_initialize(&h, &cpg_callbacks_domain);
 	if (error != CPG_OK) {
 		log_error("cpg_initialize error %d", error);
 		goto fail_free;
@@ -1370,7 +1555,7 @@ int fd_join(struct fd *fd)
 
 	cpg_fd_get(h, &f);
 
-	ci = client_add(f, process_fd_cpg, NULL);
+	ci = client_add(f, process_cpg_domain, NULL);
 
 	list_add(&fd->list, &domains);
 	fd->cpg_handle = h;
@@ -1614,28 +1799,79 @@ static void receive_protocol(struct fd_header *hd, int len)
 		return;
 	}
 
-	/* if we have zero run values, and this msg has non-zero run values,
-	   then adopt them as ours; otherwise save this proto message */
+	/* save this node's proto so we can tell when we've got all, and
+	   use it to select a minimum protocol from all */
 
-	if (our_protocol.daemon_run[0])
+	node = get_node_daemon(hd->nodeid);
+	if (!node) {
+		log_error("receive_protocol no node %d", hd->nodeid);
 		return;
+	}
 
-	if (p->daemon_run[0]) {
-		memcpy(&our_protocol.daemon_run, &p->daemon_run,
-		       sizeof(struct protocol_version));
-		log_debug("run protocol from nodeid %d", hd->nodeid);
+	if (!node->is_member) {
+		log_error("receive_protocol node %d not member", hd->nodeid);
 		return;
 	}
 
-	/* save this node's proto so we can tell when we've got all, and
-	   use it to select a minimum protocol from all */
+	log_debug("receive_protocol from %d max %u.%u.%u.%x run %u.%u.%u.%x",
+		  hd->nodeid,
+		  p->daemon_max[0], p->daemon_max[1],
+		  p->daemon_max[2], p->daemon_max[3],
+		  p->daemon_run[0], p->daemon_run[1],
+		  p->daemon_run[2], p->daemon_run[3]);
+	log_debug("daemon node %d max %u.%u.%u.%x run %u.%u.%u.%x",
+		  hd->nodeid,
+		  node->proto.daemon_max[0], node->proto.daemon_max[1],
+		  node->proto.daemon_max[2], node->proto.daemon_max[3],
+		  node->proto.daemon_run[0], node->proto.daemon_run[1],
+		  node->proto.daemon_run[2], node->proto.daemon_run[3]);
+	log_debug("daemon node %d join %llu left %llu local quorum %llu",
+		  hd->nodeid,
+		  (unsigned long long)node->join_time,
+		  (unsigned long long)node->left_time,
+		  (unsigned long long)quorate_time);
+
+	/* checking zero node->daemon_max[0] is a way to tell if we've received
+	   an acceptable (non-stateful) proto message from the node since we
+	   saw it join the daemon cpg */
+
+	if (hd->nodeid != our_nodeid &&
+	    !node->proto.daemon_max[0] &&
+	    (p->dr_ver.flags & PV_STATEFUL) &&
+	    (our_protocol.dr_ver.flags & PV_STATEFUL)) {
+
+		log_debug("daemon node %d stateful merge", hd->nodeid);
+
+		if (cman_quorate && node->left_time &&
+		    quorate_time < node->left_time) {
+			log_debug("daemon node %d kill due to stateful merge",
+				  hd->nodeid);
+			if (!node->killed)
+				kick_node_from_cluster(hd->nodeid);
+			node->killed = 1;
+		}
+
+		/* don't save p->proto into node->proto; we need to come
+		   through here based on zero daemon_max[0] for other proto
+		   messages like this one from the same node */
 
-	node = get_node_daemon(hd->nodeid);
-	if (!node) {
-		log_error("receive_protocol no node %d", hd->nodeid);
 		return;
 	}
+
 	memcpy(&node->proto, p, sizeof(struct protocol));
+
+	/* if we have zero run values, and this msg has non-zero run values,
+	   then adopt them as ours; otherwise save this proto message */
+
+	if (our_protocol.daemon_run[0])
+		return;
+
+	if (p->daemon_run[0]) {
+		our_protocol.daemon_run[0] = p->daemon_run[0];
+		our_protocol.daemon_run[1] = p->daemon_run[1];
+		our_protocol.daemon_run[2] = p->daemon_run[2];
+		log_debug("run protocol from nodeid %d", hd->nodeid);
+	}
 }
 
 static void send_protocol(struct protocol *proto)
@@ -1673,7 +1909,7 @@ int set_protocol(void)
 	int rv;
 
 	memset(&pollfd, 0, sizeof(pollfd));
-	pollfd.fd = daemon_cpg_fd;
+	pollfd.fd = cpg_fd_daemon;
 	pollfd.events = POLLIN;
 
 	while (1) {
@@ -1716,7 +1952,7 @@ int set_protocol(void)
 		}
 
 		if (pollfd.revents & POLLIN)
-			process_cpg(0);
+			process_cpg_daemon(0);
 		if (pollfd.revents & (POLLERR | POLLHUP | POLLNVAL)) {
 			log_error("set_protocol poll revents %u",
 				  pollfd.revents);
@@ -1743,14 +1979,17 @@ int set_protocol(void)
 		  our_protocol.daemon_max[0],
 		  our_protocol.daemon_max[1],
 		  our_protocol.daemon_max[2]);
+
+	send_protocol(&our_protocol);
 	return 0;
 }
 
-/* process_cpg(), setup_cpg(), close_cpg() are for the "daemon" cpg which
-   tracks the presence of other daemons; it's not the fenced domain cpg.
-   Joining this cpg tells others that we don't have uncontrolled dlm/gfs
-   kernel state and they can skip fencing us if we're a victim.  (We have
-   to check for that uncontrolled state before calling setup_cpg, obviously.) */
+/* process_cpg_daemon(), setup_cpg_daemon(), close_cpg_daemon() are for the
+   "daemon" cpg which tracks the presence of other daemons; it's not the
+   fence domain cpg.  Joining this cpg tells others that we don't have
+   uncontrolled dlm/gfs kernel state and they can skip fencing us if we're
+   a victim.  (We have to check for that uncontrolled state before calling
+   setup_cpg_daemon, obviously.) */
 
 static void deliver_cb_daemon(cpg_handle_t handle,
 			      const struct cpg_name *group_name,
@@ -1776,6 +2015,17 @@ static void deliver_cb_daemon(cpg_handle_t handle,
 	}
 }
 
+static int in_daemon_member_list(int nodeid)
+{
+	int i;
+
+	for (i = 0; i < daemon_member_count; i++) {
+		if (daemon_member[i].nodeid == nodeid)
+			return 1;
+	}
+	return 0;
+}
+
 static void confchg_cb_daemon(cpg_handle_t handle,
 			      const struct cpg_name *group_name,
 			      const struct cpg_address *member_list,
@@ -1785,8 +2035,13 @@ static void confchg_cb_daemon(cpg_handle_t handle,
 			      const struct cpg_address *joined_list,
 			      size_t joined_list_entries)
 {
+	struct node_daemon *node;
 	int i;
 
+	log_config(group_name, member_list, member_list_entries,
+		   left_list, left_list_entries,
+		   joined_list, joined_list_entries);
+
 	if (joined_list_entries)
 		send_protocol(&our_protocol);
 
@@ -1795,8 +2050,29 @@ static void confchg_cb_daemon(cpg_handle_t handle,
 
 	for (i = 0; i < member_list_entries; i++) {
 		daemon_member[i] = member_list[i];
+		/* add struct for nodes we've not seen before */
 		add_node_daemon(member_list[i].nodeid);
 	}
+
+	list_for_each_entry(node, &daemon_nodes, list) {
+		if (in_daemon_member_list(node->nodeid)) {
+			if (node->is_member)
+				continue;
+
+			/* node joined daemon cpg */
+			node->is_member = 1;
+			node->join_time = time(NULL);
+		} else {
+			if (!node->is_member)
+				continue;
+
+			/* node left daemon cpg */
+			node->is_member = 0;
+			node->killed = 0;
+			memset(&node->proto, 0, sizeof(struct protocol));
+			node->left_time = time(NULL);
+		}
+	}
 }
 
 static cpg_callbacks_t cpg_callbacks_daemon = {
@@ -1804,7 +2080,7 @@ static cpg_callbacks_t cpg_callbacks_daemon = {
 	.cpg_confchg_fn = confchg_cb_daemon,
 };
 
-void process_cpg(int ci)
+void process_cpg_daemon(int ci)
 {
 	cpg_error_t error;
 
@@ -1813,20 +2089,41 @@ void process_cpg(int ci)
 		log_error("daemon cpg_dispatch error %d", error);
 }
 
-int in_daemon_member_list(int nodeid)
+/* For a new properly started node, it will join the daemon cpg then send a
+   proto message indicating it's not stateful.  For a node merged after a
+   partition, it will join the daemon cpg and then send a proto message
+   indicating it's stateful.  To skip fencing a node we need to know it's
+   joining after starting cleanly, so we need to see the daemon cpg joined
+   followed by a proto message without the STATEFUL flag. */
+
+int is_clean_daemon_member(int nodeid)
 {
-	int i;
+	struct node_daemon *node;
+
+	/* process confchg's and protocol messages for daemon cpg,
+	   need to dispatch here because this is called from fence_victims()
+	   outside the poll() loop */
 
 	cpg_dispatch(cpg_handle_daemon, CPG_DISPATCH_ALL);
 
-	for (i = 0; i < daemon_member_count; i++) {
-		if (daemon_member[i].nodeid == nodeid)
+	node = get_node_daemon(nodeid);
+	if (node && node->is_member) {
+		/* have we received a non-stateful proto message from it?
+		   if so then daemon_max[0] will be non-zero */
+		if (node->proto.daemon_max[0]) {
+			/* assert !(node->proto.dr_ver.flags & PV_STATEFUL) ? */
+			log_debug("daemon_member %d is clean", nodeid);
 			return 1;
+		} else {
+			log_debug("daemon_member %d zero proto", nodeid);
+			return 0;
+		}
 	}
+	log_debug("daemon_member %d not member", nodeid);
 	return 0;
 }
 
-int setup_cpg(void)
+int setup_cpg_daemon(void)
 {
 	cpg_error_t error;
 	struct cpg_name name;
@@ -1845,7 +2142,7 @@ int setup_cpg(void)
 		goto ret;
 	}
 
-	cpg_fd_get(cpg_handle_daemon, &daemon_cpg_fd);
+	cpg_fd_get(cpg_handle_daemon, &cpg_fd_daemon);
 
 	memset(&name, 0, sizeof(name));
 	sprintf(name.value, "fenced:daemon");
@@ -1864,8 +2161,8 @@ int setup_cpg(void)
 		goto fail;
 	}
 
-	log_debug("setup_cpg %d", daemon_cpg_fd);
-	return daemon_cpg_fd;
+	log_debug("setup_cpg_daemon %d", cpg_fd_daemon);
+	return cpg_fd_daemon;
 
  fail:
 	cpg_finalize(cpg_handle_daemon);
@@ -1873,7 +2170,7 @@ int setup_cpg(void)
 	return -1;
 }
 
-void close_cpg(void)
+void close_cpg_daemon(void)
 {
 	struct fd *fd;
 	cpg_error_t error;
diff --git a/fence/fenced/fd.h b/fence/fenced/fd.h
index 22ffe2c..d16a49b 100644
--- a/fence/fenced/fd.h
+++ b/fence/fenced/fd.h
@@ -64,6 +64,7 @@ extern int daemon_quit;
 extern int cluster_down;
 extern struct list_head domains;
 extern int cman_quorate;
+extern uint64_t quorate_time;
 extern int our_nodeid;
 extern char our_name[MAX_NODENAME_LEN+1];
 extern char daemon_debug_buf[256];
@@ -122,6 +123,7 @@ struct change {
 	int state; /* CGST_ */
 	int we_joined;
 	uint32_t seq; /* just used as a reference when debugging */
+	uint64_t create_time;
 };
 
 #define VIC_DONE_AGENT		1
@@ -138,6 +140,8 @@ struct node_history {
 	uint64_t fail_time;
 	uint64_t fence_time;
 	uint64_t fence_external_time;
+	uint64_t cman_add_time;
+	uint64_t cman_remove_time;
 	int fence_external_node;
 	int fence_master;
 	int fence_how; /* VIC_DONE_ */
@@ -147,6 +151,7 @@ struct node {
 	struct list_head 	list;
 	int			nodeid;
 	int			init_victim;
+	int			local_victim_done;
 	char 			name[MAX_NODENAME_LEN+1];
 };
 
@@ -165,6 +170,7 @@ struct fd {
 	struct list_head	changes;
 	struct list_head	node_history;
 	int			init_complete;
+	int			local_init_complete;
 
 	/* general domain membership */
 
@@ -198,9 +204,9 @@ int read_ccs(struct fd *fd);
 
 /* cpg.c */
 
-void process_cpg(int ci);
-int setup_cpg(void);
-void close_cpg(void);
+void process_cpg_daemon(int ci);
+int setup_cpg_daemon(void);
+void close_cpg_daemon(void);
 int set_protocol(void);
 void free_cg(struct change *cg);
 void node_history_init(struct fd *fd, int nodeid);
@@ -216,7 +222,9 @@ int set_node_info(struct fd *fd, int nodeid, struct fenced_node *node);
 int set_domain_info(struct fd *fd, struct fenced_domain *domain);
 int set_domain_nodes(struct fd *fd, int option, int *node_count,
 		     struct fenced_node **nodes);
-int in_daemon_member_list(int nodeid);
+int is_clean_daemon_member(int nodeid);
+void node_history_cman_add(int nodeid);
+void node_history_cman_remove(int nodeid);
 
 /* group.c */
 
@@ -246,7 +254,7 @@ void cluster_dead(int ci);
 void process_cman(int ci);
 int setup_cman(void);
 void close_cman(void);
-int is_cman_member(int nodeid);
+int is_cman_member_reread(int nodeid);
 char *nodeid_to_name(int nodeid);
 int name_to_nodeid(char *name);
 struct node *get_new_node(struct fd *fd, int nodeid);
diff --git a/fence/fenced/group.c b/fence/fenced/group.c
index 2f95550..0f85e85 100644
--- a/fence/fenced/group.c
+++ b/fence/fenced/group.c
@@ -173,7 +173,7 @@ static void _add_first_victims(struct fd *fd)
 		log_debug("first complete list empty warning");
 
 	list_for_each_entry_safe(prev_node, safe, &fd->complete, list) {
-		if (!is_cman_member(prev_node->nodeid)) {
+		if (!is_cman_member_reread(prev_node->nodeid)) {
 			list_del(&prev_node->list);
 			list_add(&prev_node->list, &fd->victims);
 			log_debug("add first victim %s", prev_node->name);
@@ -381,8 +381,10 @@ int fd_join_group(struct fd *fd)
 		log_error("group_join error %d", rv);
 		list_del(&fd->list);
 		free(fd);
+		return rv;
 	}
-	return rv;
+	set_cman_dirty();
+	return 0;
 }
 
 int fd_leave_group(struct fd *fd)
diff --git a/fence/fenced/main.c b/fence/fenced/main.c
index 5430afe..9e473d2 100644
--- a/fence/fenced/main.c
+++ b/fence/fenced/main.c
@@ -189,12 +189,13 @@ struct fd *find_fd(char *name)
 
 /* We don't require cman dirty/disallowed to detect and handle cpg merges after
    a partition, because we already do that with started_count checks and our
-   own disallowed flag.  But, we do need cman dirty/disallowed to deal with
-   correctly skipping victims that rejoin the cluster.  Without cman
-   dirty/disallowed, we'd skip fencing a node after a merge of a partition
-   since the merged node would be a cman member and a fenced:daemon cpg member.
-   By setting the dirty flag, cman won't report a dirty merged node as a
-   member, so we'll continue fencing it. */
+   own disallowed flag.  We also don't require cman dirty/disallowed to deal
+   with correctly skipping victims that rejoin the cluster.  If a victim
+   joins the fenced:daemon cpg and sends a proto message without the STATEFUL
+   flag, then we safely skip fencing it.  This is what happens when the node
+   starts cleanly.  If a node is added back after a partition, we see it join
+   the fenced:daemon cpg and it send a proto message with the STATEFUL flag set.
+   In this case it remains a victim and we will continue with fencing it. */
 
 static int do_join(char *name)
 {
@@ -224,9 +225,6 @@ static int do_join(char *name)
 		rv = fd_join_group(fd);
 	else
 		rv = fd_join(fd);
-
-	if (!rv)
-		set_cman_dirty();
  out:
 	return rv;
 }
@@ -681,7 +679,7 @@ static int check_controlled_dir(char *path)
 	return count;
 }
 
-/* Joining the "fenced:daemon" cpg (in setup_cpg()) tells fenced on other
+/* Joining the "fenced:daemon" cpg (in setup_cpg_daemon()) tells fenced on other
    nodes that we are in a "clean state", and don't need fencing.  So, if
    we're a pending fence victim on another node, they'll skip fencing us
    once we start fenced and join the "daemon" cpg (it's not the fence domain
@@ -758,10 +756,10 @@ static void loop(void)
 	if (rv < 0)
 		goto out;
 
-	rv = setup_cpg();
+	rv = setup_cpg_daemon();
 	if (rv < 0)
 		goto out;
-	client_add(rv, process_cpg, cluster_dead);
+	client_add(rv, process_cpg_daemon, cluster_dead);
 
 	group_mode = GROUP_LIBCPG;
 
@@ -830,7 +828,7 @@ static void loop(void)
  out:
 	if (cfgd_groupd_compat)
 		close_groupd();
-	close_cpg();
+	close_cpg_daemon();
 	close_logging();
 	close_ccs();
 	close_cman();
@@ -1071,6 +1069,7 @@ int daemon_quit;
 int cluster_down;
 struct list_head domains;
 int cman_quorate;
+uint64_t quorate_time;
 int our_nodeid;
 char our_name[MAX_NODENAME_LEN+1];
 char daemon_debug_buf[256];
diff --git a/fence/fenced/member_cman.c b/fence/fenced/member_cman.c
index 83de8f1..a621173 100644
--- a/fence/fenced/member_cman.c
+++ b/fence/fenced/member_cman.c
@@ -7,6 +7,8 @@
 
 static cman_handle_t	ch;
 static cman_handle_t	ch_admin;
+static cman_node_t	old_nodes[MAX_NODES];
+static int		old_node_count;
 static cman_node_t	cman_nodes[MAX_NODES];
 static int		cman_node_count;
 
@@ -31,6 +33,27 @@ void kick_node_from_cluster(int nodeid)
 	}
 }
 
+static int is_member(cman_node_t *node_list, int count, int nodeid)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		if (node_list[i].cn_nodeid == nodeid)
+			return node_list[i].cn_member;
+	}
+	return 0;
+}
+
+static int is_old_member(int nodeid)
+{
+	return is_member(old_nodes, old_node_count, nodeid);
+}
+
+static int is_cman_member(int nodeid)
+{
+	return is_member(cman_nodes, cman_node_count, nodeid);
+}
+
 static int name_equal(char *name1, char *name2)
 {
 	char name3[BUFLEN], name4[BUFLEN];
@@ -125,15 +148,46 @@ int name_to_nodeid(char *name)
 
 static void statechange(void)
 {
-	int rv;
+	int quorate = cman_quorate;
+	int i, rv;
 
 	cman_quorate = cman_is_quorate(ch);
+
+	if (!quorate && cman_quorate)
+		quorate_time = time(NULL);
+
+	old_node_count = cman_node_count;
+	memcpy(&old_nodes, &cman_nodes, sizeof(old_nodes));
+
 	cman_node_count = 0;
 	memset(&cman_nodes, 0, sizeof(cman_nodes));
-
 	rv = cman_get_nodes(ch, MAX_NODES, &cman_node_count, cman_nodes);
-	if (rv < 0)
+	if (rv < 0) {
 		log_error("cman_get_nodes error %d %d", rv, errno);
+		return;
+	}
+
+	for (i = 0; i < old_node_count; i++) {
+		if (old_nodes[i].cn_member &&
+		    !is_cman_member(old_nodes[i].cn_nodeid)) {
+
+			log_debug("cman node %d removed",
+				  old_nodes[i].cn_nodeid);
+
+			node_history_cman_remove(old_nodes[i].cn_nodeid);
+		}
+	}
+
+	for (i = 0; i < cman_node_count; i++) {
+		if (cman_nodes[i].cn_member &&
+		    !is_old_member(cman_nodes[i].cn_nodeid)) {
+
+			log_debug("cman node %d added",
+				  cman_nodes[i].cn_nodeid);
+
+			node_history_cman_add(cman_nodes[i].cn_nodeid);
+		}
+	}
 }
 
 static void cman_callback(cman_handle_t h, void *private, int reason, int arg)
@@ -248,19 +302,19 @@ void close_cman(void)
 	cman_finish(ch_admin);
 }
 
-int is_cman_member(int nodeid)
+int is_cman_member_reread(int nodeid)
 {
-	cman_node_t *cn;
+	int rv;
 
 	/* Note: in fence delay loop we aren't processing callbacks so won't
 	   have done a statechange() in response to a cman callback */
 	statechange();
 
-	cn = find_cman_node(nodeid);
-	if (cn && cn->cn_member)
+	rv = is_cman_member(nodeid);
+	if (rv)
 		return 1;
 
-	log_debug("node %d not a cman member, cn %d", nodeid, cn ? 1 : 0);
+	log_debug("cman_member %d not member", nodeid);
 	return 0;
 }
 
diff --git a/fence/fenced/recover.c b/fence/fenced/recover.c
index e956e83..aa1cfe5 100644
--- a/fence/fenced/recover.c
+++ b/fence/fenced/recover.c
@@ -67,8 +67,8 @@ static int reduce_victims(struct fd *fd)
 	num_victims = list_count(&fd->victims);
 
 	list_for_each_entry_safe(node, safe, &fd->victims, list) {
-		if (is_cman_member(node->nodeid) &&
-		    in_daemon_member_list(node->nodeid)) {
+		if (is_cman_member_reread(node->nodeid) &&
+		    is_clean_daemon_member(node->nodeid)) {
 			log_debug("reduce victim %s", node->name);
 			victim_done(fd, node->nodeid, VIC_DONE_MEMBER);
 			list_del(&node->list);
@@ -286,14 +286,30 @@ void fence_victims(struct fd *fd)
 	int override = -1;
 	int cman_member, cpg_member, ext;
 
-	while (!list_empty(&fd->victims)) {
-		node = list_entry(fd->victims.next, struct node, list);
-
+	list_for_each_entry(node, &fd->victims, list) {
+		if (node->local_victim_done) {
+			/* local_victim_done means we've successfully fenced
+			   this node and will remove it from victims list
+			   upon receipt of the victim_done message we sent */
+
+			/* I don't believe fence_victims() can come across
+			   a node with local_victim_done set.  That would
+			   mean: pass 1 through fence_victims() fences node A,
+			   send victim_done(A), return, process confchgs
+			   adding and removing A again, receive msgs for start
+			   cycle, but *not* receive victim_done(A) msg before
+			   coming through fence_victims() again. */
+
+			log_error("skip local_victim_done node %d",
+				  node->nodeid);
+			continue;
+		}
+ retry:
 		/* for queries */
 		fd->current_victim = node->nodeid;
 
-		cman_member = is_cman_member(node->nodeid);
-		cpg_member = in_daemon_member_list(node->nodeid);
+		cman_member = is_cman_member_reread(node->nodeid);
+		cpg_member = is_clean_daemon_member(node->nodeid);
 		if (group_mode == GROUP_LIBCPG)
 			ext = is_fenced_external(fd, node->nodeid);
 		else
@@ -303,10 +319,10 @@ void fence_victims(struct fd *fd)
 			log_debug("averting fence of node %s "
 				  "cman member %d cpg member %d external %d",
 				  node->name, cman_member, cpg_member, ext);
+
+			node->local_victim_done = 1;
 			victim_done(fd, node->nodeid,
 				    ext ? VIC_DONE_EXTERNAL : VIC_DONE_MEMBER);
-			list_del(&node->list);
-			free(node);
 			continue;
 		}
 
@@ -339,9 +355,8 @@ void fence_victims(struct fd *fd)
 			  error ? "failed" : "success");
 
 		if (!error) {
+			node->local_victim_done = 1;
 			victim_done(fd, node->nodeid, VIC_DONE_AGENT);
-			list_del(&node->list);
-			free(node);
 			continue;
 		}
 
@@ -349,7 +364,7 @@ void fence_victims(struct fd *fd)
 			query_unlock();
 			sleep(5);
 			query_lock();
-			continue;
+			goto retry;
 		}
 
 		/* Check for manual intervention */
@@ -358,11 +373,14 @@ void fence_victims(struct fd *fd)
 				   cfgd_override_time) > 0) {
 			log_level(LOG_WARNING, "fence %s overridden by "
 				  "administrator intervention", node->name);
+			node->local_victim_done = 1;
 			victim_done(fd, node->nodeid, VIC_DONE_OVERRIDE);
-			list_del(&node->list);
-			free(node);
+			close_override(&override, cfgd_override_path);
+			continue;
+		} else {
+			close_override(&override, cfgd_override_path);
+			goto retry;
 		}
-		close_override(&override, cfgd_override_path);
 	}
 
 	fd->current_victim = 0;


More information about the cluster-commits mailing list