[SCM] Samba Shared Repository - branch master updated

Martin Schwenke martins at samba.org
Fri Jul 24 06:04:03 UTC 2020


The branch, master has been updated
       via  5ce6133a751 ctdb-recoverd: Simplify calculation of new flags
       via  3654e416770 ctdb-recoverd: Correctly find nodemap entry for pnn
       via  9475ab04416 ctdb-recoverd: Do not retrieve nodemap from recovery master
       via  0c6a7db3ba8 ctdb-recoverd: Flatten update_flags_on_all_nodes()
       via  a88c10c5a9a ctdb-recoverd: Move ctdb_ctrl_modflags() to ctdb_recoverd.c
       via  b1e631ff929 ctdb-recoverd: Improve a call to update_flags_on_all_nodes()
       via  915d24ac12d ctdb-recoverd: Use update_flags_on_all_nodes()
       via  f681c0e9477 ctdb-recoverd: Introduce some local variables to improve readability
       via  cb3a3147b7a ctdb-recoverd: Change update_flags_on_all_nodes() to take rec argument
       via  6982fcb3e6c ctdb-recoverd: Drop unused nodemap argument from update_flags_on_all_nodes()
      from  484a764e832 ctdb-tests: Improve test portability/quality

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 5ce6133a75107abdcb9fcfd93bc7594812dc5055
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Jul 14 14:29:09 2020 +1000

    ctdb-recoverd: Simplify calculation of new flags
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>
    
    Autobuild-User(master): Martin Schwenke <martins at samba.org>
    Autobuild-Date(master): Fri Jul 24 06:03:23 UTC 2020 on sn-devel-184

commit 3654e416770cc7521dcc3c15976daeba37023304
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Jul 14 14:22:15 2020 +1000

    ctdb-recoverd: Correctly find nodemap entry for pnn
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit 9475ab044161e687b9ced3a477746393565b49b1
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue May 5 23:49:05 2020 +1000

    ctdb-recoverd: Do not retrieve nodemap from recovery master
    
    It is already in rec->nodemap.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit 0c6a7db3ba84b8355359b0a8c52690b234bb866d
Author: Martin Schwenke <martin at meltin.net>
Date:   Fri Sep 28 10:46:17 2018 +1000

    ctdb-recoverd: Flatten update_flags_on_all_nodes()
    
    The logic currently in ctdb_ctrl_modflags() will be optimised so that
    it no longer matches the pattern for a control function.  So, remove
    this function and squash its functionality into the only caller.
    
    Although there are some superficial changes, the behaviour is
    unchanged.
    
    Flattening the 2 functions produces some seriously weird logic for
    setting the new flags, to the point where using ctdb_ctrl_modflags()
    for this purpose now looks very strange.  The weirdness will be
    cleaned up in a subsequent commit.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit a88c10c5a9afcf0a3dcadef07dd95af498bfa47a
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue May 5 23:37:57 2020 +1000

    ctdb-recoverd: Move ctdb_ctrl_modflags() to ctdb_recoverd.c
    
    This file is the only user of this function.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit b1e631ff929fd87392a80895d1c8d265d9df42dc
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Jul 14 14:43:04 2020 +1000

    ctdb-recoverd: Improve a call to update_flags_on_all_nodes()
    
    This should take a PNN, not an array index.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit 915d24ac12d27c21649d9e64d201d9df9d583129
Author: Martin Schwenke <martin at meltin.net>
Date:   Sat Jun 15 07:20:19 2019 +1000

    ctdb-recoverd: Use update_flags_on_all_nodes()
    
    This is clearer than using the MODFLAGS control directly.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit f681c0e947741151f8fb95d88edddfd732166dc1
Author: Martin Schwenke <martin at meltin.net>
Date:   Sat Jun 15 07:19:26 2019 +1000

    ctdb-recoverd: Introduce some local variables to improve readability
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit cb3a3147b7a3a29d7806733791e1fa6ba2e46680
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue May 5 23:45:15 2020 +1000

    ctdb-recoverd: Change update_flags_on_all_nodes() to take rec argument
    
    This makes fields such as recmaster and nodemap easily available if
    required.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

commit 6982fcb3e6c940d0047aac3b6bfbc9dfdc8d7214
Author: Martin Schwenke <martin at meltin.net>
Date:   Thu Jan 18 20:25:07 2018 +1100

    ctdb-recoverd: Drop unused nodemap argument from update_flags_on_all_nodes()
    
    An unused argument needlessly extends the length of function calls.  A
    subsequent change will allow rec->nodemap to be used if necessary.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>
    Reviewed-by: Amitay Isaacs <amitay at gmail.com>

-----------------------------------------------------------------------

Summary of changes:
 ctdb/include/ctdb_client.h  |   5 ---
 ctdb/server/ctdb_client.c   |  65 ---------------------------
 ctdb/server/ctdb_recoverd.c | 107 +++++++++++++++++++++++++++++++++-----------
 3 files changed, 82 insertions(+), 95 deletions(-)


Changeset truncated at 500 lines:

diff --git a/ctdb/include/ctdb_client.h b/ctdb/include/ctdb_client.h
index 198a8a38dbb..b89c4e49b2f 100644
--- a/ctdb/include/ctdb_client.h
+++ b/ctdb/include/ctdb_client.h
@@ -195,11 +195,6 @@ int ctdb_ctrl_get_ifaces(struct ctdb_context *ctdb,
 			 TALLOC_CTX *mem_ctx,
 			 struct ctdb_iface_list_old **ifaces);
 
-int ctdb_ctrl_modflags(struct ctdb_context *ctdb,
-		       struct timeval timeout,
-		       uint32_t destnode,
-		       uint32_t set, uint32_t clear);
-
 int ctdb_ctrl_get_all_tunables(struct ctdb_context *ctdb,
 			       struct timeval timeout, uint32_t destnode,
 			       struct ctdb_tunable_list *tunables);
diff --git a/ctdb/server/ctdb_client.c b/ctdb/server/ctdb_client.c
index 453e7b28477..5d1a30d03da 100644
--- a/ctdb/server/ctdb_client.c
+++ b/ctdb/server/ctdb_client.c
@@ -1243,71 +1243,6 @@ int ctdb_ctrl_get_ifaces(struct ctdb_context *ctdb,
 	return 0;
 }
 
-/*
-  set/clear the permanent disabled bit on a remote node
- */
-int ctdb_ctrl_modflags(struct ctdb_context *ctdb, struct timeval timeout, uint32_t destnode,
-		       uint32_t set, uint32_t clear)
-{
-	int ret;
-	TDB_DATA data;
-	struct ctdb_node_map_old *nodemap=NULL;
-	struct ctdb_node_flag_change c;
-	TALLOC_CTX *tmp_ctx = talloc_new(ctdb);
-	uint32_t recmaster;
-	uint32_t *nodes;
-
-
-	/* find the recovery master */
-	ret = ctdb_ctrl_getrecmaster(ctdb, tmp_ctx, timeout, CTDB_CURRENT_NODE, &recmaster);
-	if (ret != 0) {
-		DEBUG(DEBUG_ERR, (__location__ " Unable to get recmaster from local node\n"));
-		talloc_free(tmp_ctx);
-		return ret;
-	}
-
-
-	/* read the node flags from the recmaster */
-	ret = ctdb_ctrl_getnodemap(ctdb, timeout, recmaster, tmp_ctx, &nodemap);
-	if (ret != 0) {
-		DEBUG(DEBUG_ERR, (__location__ " Unable to get nodemap from node %u\n", destnode));
-		talloc_free(tmp_ctx);
-		return -1;
-	}
-	if (destnode >= nodemap->num) {
-		DEBUG(DEBUG_ERR,(__location__ " Nodemap from recmaster does not contain node %d\n", destnode));
-		talloc_free(tmp_ctx);
-		return -1;
-	}
-
-	c.pnn       = destnode;
-	c.old_flags = nodemap->nodes[destnode].flags;
-	c.new_flags = c.old_flags;
-	c.new_flags |= set;
-	c.new_flags &= ~clear;
-
-	data.dsize = sizeof(c);
-	data.dptr = (unsigned char *)&c;
-
-	/* send the flags update to all connected nodes */
-	nodes = list_of_connected_nodes(ctdb, nodemap, tmp_ctx, true);
-
-	if (ctdb_client_async_control(ctdb, CTDB_CONTROL_MODIFY_FLAGS,
-					nodes, 0,
-					timeout, false, data,
-					NULL, NULL,
-					NULL) != 0) {
-		DEBUG(DEBUG_ERR, (__location__ " Unable to update nodeflags on remote nodes\n"));
-
-		talloc_free(tmp_ctx);
-		return -1;
-	}
-
-	talloc_free(tmp_ctx);
-	return 0;
-}
-
-
 /*
   get all tunables
  */
diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c
index 3f5d43c1e87..3f72619127a 100644
--- a/ctdb/server/ctdb_recoverd.c
+++ b/ctdb/server/ctdb_recoverd.c
@@ -425,18 +425,62 @@ static int set_recovery_mode(struct ctdb_context *ctdb,
 }
 
 /*
-  update flags on all active nodes
+ * Update flags on all connected nodes
  */
-static int update_flags_on_all_nodes(struct ctdb_context *ctdb, struct ctdb_node_map_old *nodemap, uint32_t pnn, uint32_t flags)
+static int update_flags_on_all_nodes(struct ctdb_recoverd *rec,
+				     uint32_t pnn,
+				     uint32_t flags)
 {
+	struct ctdb_context *ctdb = rec->ctdb;
+	struct timeval timeout = CONTROL_TIMEOUT();
+	TDB_DATA data;
+	struct ctdb_node_map_old *nodemap=NULL;
+	struct ctdb_node_flag_change c;
+	TALLOC_CTX *tmp_ctx = talloc_new(ctdb);
+	uint32_t *nodes;
+	uint32_t i;
 	int ret;
 
-	ret = ctdb_ctrl_modflags(ctdb, CONTROL_TIMEOUT(), pnn, flags, ~flags);
-		if (ret != 0) {
-		DEBUG(DEBUG_ERR, (__location__ " Unable to update nodeflags on remote nodes\n"));
+	nodemap = rec->nodemap;
+
+	for (i = 0; i < nodemap->num; i++) {
+		if (pnn == nodemap->nodes[i].pnn) {
+			break;
+		}
+	}
+	if (i >= nodemap->num) {
+		DBG_ERR("Nodemap does not contain node %d\n", pnn);
+		talloc_free(tmp_ctx);
 		return -1;
 	}
 
+	c.pnn       = pnn;
+	c.old_flags = nodemap->nodes[i].flags;
+	c.new_flags = flags;
+
+	data.dsize = sizeof(c);
+	data.dptr = (unsigned char *)&c;
+
+	/* send the flags update to all connected nodes */
+	nodes = list_of_connected_nodes(ctdb, nodemap, tmp_ctx, true);
+
+	ret = ctdb_client_async_control(ctdb,
+					CTDB_CONTROL_MODIFY_FLAGS,
+					nodes,
+					0,
+					timeout,
+					false,
+					data,
+					NULL,
+					NULL,
+					NULL);
+	if (ret != 0) {
+		DBG_ERR("Unable to update flags on remote nodes\n");
+		talloc_free(tmp_ctx);
+		return -1;
+	}
+
+	talloc_free(tmp_ctx);
 	return 0;
 }
 
@@ -507,9 +551,11 @@ static int update_local_flags(struct ctdb_recoverd *rec, struct ctdb_node_map_ol
 	 */
 	for (j=0; j<nodemap->num; j++) {
 		struct ctdb_node_map_old *remote_nodemap=NULL;
+		uint32_t local_flags = nodemap->nodes[j].flags;
+		uint32_t remote_flags;
 		int ret;
 
-		if (nodemap->nodes[j].flags & NODE_FLAGS_DISCONNECTED) {
+		if (local_flags & NODE_FLAGS_DISCONNECTED) {
 			continue;
 		}
 		if (nodemap->nodes[j].pnn == ctdb->pnn) {
@@ -525,26 +571,29 @@ static int update_local_flags(struct ctdb_recoverd *rec, struct ctdb_node_map_ol
 			talloc_free(mem_ctx);
 			return -1;
 		}
-		if (nodemap->nodes[j].flags != remote_nodemap->nodes[j].flags) {
-			/* We should tell our daemon about this so it
-			   updates its flags or else we will log the same 
-			   message again in the next iteration of recovery.
-			   Since we are the recovery master we can just as
-			   well update the flags on all nodes.
-			*/
-			ret = ctdb_ctrl_modflags(ctdb, CONTROL_TIMEOUT(), nodemap->nodes[j].pnn, remote_nodemap->nodes[j].flags, ~remote_nodemap->nodes[j].flags);
+		remote_flags = remote_nodemap->nodes[j].flags;
+
+		if (local_flags != remote_flags) {
+			ret = update_flags_on_all_nodes(rec,
+							nodemap->nodes[j].pnn,
+							remote_flags);
 			if (ret != 0) {
-				DEBUG(DEBUG_ERR, (__location__ " Unable to update nodeflags on remote nodes\n"));
+				DBG_ERR(
+				    "Unable to update flags on remote nodes\n");
+				talloc_free(mem_ctx);
 				return -1;
 			}
 
-			/* Update our local copy of the flags in the recovery
-			   daemon.
-			*/
-			DEBUG(DEBUG_NOTICE,("Remote node %u had flags 0x%x, local had 0x%x - updating local\n",
-				 nodemap->nodes[j].pnn, remote_nodemap->nodes[j].flags,
-				 nodemap->nodes[j].flags));
-			nodemap->nodes[j].flags = remote_nodemap->nodes[j].flags;
+			/*
+			 * Update the local copy of the flags in the
+			 * recovery daemon.
+			 */
+			D_NOTICE("Remote node %u had flags 0x%x, "
+				 "local had 0x%x - updating local\n",
+				 nodemap->nodes[j].pnn,
+				 remote_flags,
+				 local_flags);
+			nodemap->nodes[j].flags = remote_flags;
 		}
 		talloc_free(remote_nodemap);
 	}
@@ -1125,7 +1174,9 @@ static int do_recovery(struct ctdb_recoverd *rec,
 			continue;
 		}
 
-		ret = update_flags_on_all_nodes(ctdb, nodemap, i, nodemap->nodes[i].flags);
+		ret = update_flags_on_all_nodes(rec,
+						nodemap->nodes[i].pnn,
+						nodemap->nodes[i].flags);
 		if (ret != 0) {
 			if (nodemap->nodes[i].flags & NODE_FLAGS_INACTIVE) {
 				DEBUG(DEBUG_WARNING, (__location__ "Unable to update flags on inactive node %d\n", i));
@@ -2610,14 +2661,20 @@ static void main_loop(struct ctdb_context *ctdb, struct ctdb_recoverd *rec,
 				  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(ctdb, nodemap, nodemap->nodes[i].pnn, remote_nodemaps[j]->nodes[i].flags);
+					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(ctdb, nodemap, nodemap->nodes[i].pnn, nodemap->nodes[i].flags);
+					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);


-- 
Samba Shared Repository



More information about the samba-cvs mailing list