Attachment 16280 Details for Bug 14513 – Patch for 4.11

[patch] Patch for 4.11

BZ14513-v4-11.patch (text/plain), 4.79 KB, created by Martin Schwenke on 2020-10-12 05:42:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

Created: 2020-10-12 05:42:33 UTC

Size: 4.79 KB

patch

obsolete

>From b2a196557f0f2b1b3d9d7c7d0e54d6a433116761 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 19 Jan 2018 14:55:21 +1100
>Subject: [PATCH 1/2] ctdb-recoverd: Drop unnecessary code
>
>This has already been done in update_flags().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14513
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit 3ab52b528673e08caa66f00e963528c591a84fe1)
>---
> ctdb/server/ctdb_recoverd.c | 14 --------------
> 1 file changed, 14 deletions(-)
>
>diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c
>index 7c53335604a..2aee9f056c7 100644
>--- a/ctdb/server/ctdb_recoverd.c
>+++ b/ctdb/server/ctdb_recoverd.c
>@@ -2667,20 +2667,6 @@ static void main_loop(struct ctdb_context *ctdb, struct ctdb_recoverd *rec,
> 		}
> 	}
> 
>-	/*
>-	 * Update node flags obtained from each active node. This ensure we have
>-	 * up-to-date information for all the nodes.
>-	 */
>-	for (j=0; j<nodemap->num; j++) {
>-		if (nodemap->nodes[j].pnn == ctdb->pnn) {
>-			continue;
>-		}
>-		if (nodemap->nodes[j].flags & NODE_FLAGS_INACTIVE) {
>-			continue;
>-		}
>-		nodemap->nodes[j].flags = remote_nodemaps[j]->nodes[j].flags;
>-	}
>-
> 	for (j=0; j<nodemap->num; j++) {
> 		if (nodemap->nodes[j].pnn == ctdb->pnn) {
> 			continue;
>-- 
>2.28.0
>
>
>From 0557dcdeefe25e0e2ca7cd0c18fe2a7ede2968d9 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Tue, 16 Jan 2018 15:15:51 +1100
>Subject: [PATCH 2/2] ctdb-recoverd: Drop unnecessary and broken code
>
>update_flags() has already updated the recovery master's canonical
>node map, based on the flags from each remote node, and pushed out
>these flags to all nodes.
>
>If i == j then the node map has already been updated from this remote
>node's flags, so simply drop this case.
>
>Although update_flags() has updated flags for all nodes, it did not
>update each node map in remote_nodemaps[] to reflect this.  This means
>that remote_nodemaps[] may contain inconsistent flags for some nodes
>so it should not be used to check consistency when i != j.
>
>Further, a meaningful difference in flags can only really occur if
>update_flags() failed.  In that case this code is never reached.
>
>These observations combine to imply that this whole loop should be
>dropped.
>
>This leaves potential sub-second inconsistencies due to out-of-band
>healthy/unhealthy flag changes pushed via CTDB_SRVID_PUSH_NODE_FLAGS.
>These updates could be dropped (takeover run asks each node for
>available IPs rather than making centralised decisions based on node
>flags) but for now they will be fixed in the next iteration of
>main_loop().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14513
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit 4b01f54041dee469971f244e64064eed46de2ed5)
>---
> ctdb/server/ctdb_recoverd.c | 47 -------------------------------------
> 1 file changed, 47 deletions(-)
>
>diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c
>index 2aee9f056c7..8501fe877ff 100644
>--- a/ctdb/server/ctdb_recoverd.c
>+++ b/ctdb/server/ctdb_recoverd.c
>@@ -2667,53 +2667,6 @@ static void main_loop(struct ctdb_context *ctdb, struct ctdb_recoverd *rec,
> 		}
> 	}
> 
>-	for (j=0; j<nodemap->num; j++) {
>-		if (nodemap->nodes[j].pnn == ctdb->pnn) {
>-			continue;
>-		}
>-		if (nodemap->nodes[j].flags & NODE_FLAGS_INACTIVE) {
>-			continue;
>-		}
>-
>-		/* verify the flags are consistent
>-		*/
>-		for (i=0; i<nodemap->num; i++) {
>-			if (nodemap->nodes[i].flags & NODE_FLAGS_DISCONNECTED) {
>-				continue;
>-			}
>-			
>-			if (nodemap->nodes[i].flags != remote_nodemaps[j]->nodes[i].flags) {
>-				DEBUG(DEBUG_ERR, (__location__ " Remote node:%u has different flags for node %u. It has 0x%02x vs our 0x%02x\n", 
>-				  nodemap->nodes[j].pnn, 
>-				  nodemap->nodes[i].pnn, 
>-				  remote_nodemaps[j]->nodes[i].flags,
>-				  nodemap->nodes[i].flags));
>-				if (i == j) {
>-					DEBUG(DEBUG_ERR,("Use flags 0x%02x from remote node %d for cluster update of its own flags\n", remote_nodemaps[j]->nodes[i].flags, j));
>-					update_flags_on_all_nodes(
>-					    rec,
>-					    nodemap->nodes[i].pnn,
>-					    remote_nodemaps[j]->nodes[i].flags);
>-					ctdb_set_culprit(rec, nodemap->nodes[j].pnn);
>-					do_recovery(rec, mem_ctx, pnn, nodemap, 
>-						    vnnmap);
>-					return;
>-				} else {
>-					DEBUG(DEBUG_ERR,("Use flags 0x%02x from local recmaster node for cluster update of node %d flags\n", nodemap->nodes[i].flags, i));
>-					update_flags_on_all_nodes(
>-						rec,
>-						nodemap->nodes[i].pnn,
>-						nodemap->nodes[i].flags);
>-					ctdb_set_culprit(rec, nodemap->nodes[j].pnn);
>-					do_recovery(rec, mem_ctx, pnn, nodemap, 
>-						    vnnmap);
>-					return;
>-				}
>-			}
>-		}
>-	}
>-
>-
> 	/* count how many active nodes there are */
> 	num_lmasters  = 0;
> 	for (i=0; i<nodemap->num; i++) {
>-- 
>2.28.0
>

Flags:

amitay: review+

Actions: View

Attachments on bug 14513: 16278 | 16279 | 16280