From 4dd382296d3e78000713ab0ac1f8e531e25857cc Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 23 May 2014 21:58:55 +1000 Subject: [PATCH] ctdb-tools-ctdb: Make natgwlist and lvsmaster more resilient Recent changes have caused these commands to attempt to get capabilities from all nodes before doing further filtering. This means that capabilities are unnecessarily fetched from nodes that are unlikely to be the master. If such a node does not answer the control then many nodes can fail to calculate the master node. In the case of natgwlist this will cause "monitor" events to fail resulting in unhealthy nodes. Restore the behaviour where capabilities are only fetched for a node that will be the master if it has the desired flags. Although this masks a problem where a connected node is not replying, it can help to avoid an outage in some cases. Add supporting tests and infrastructure. Infrastructure just lets a timeout be faked - just for ctdb_ctrl_getcapabilities_stub() so far. First test checks that this infrastructure works if the first node times out in natgwlist. Second test checks the case worked around by the above fix - that is, no failure when a node with PNN beyond the NATGW master can time out. Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs Autobuild-User(master): Amitay Isaacs Autobuild-Date(master): Thu May 29 05:59:37 CEST 2014 on sn-devel-104 --- ctdb/tests/src/ctdb_test_stubs.c | 23 ++++++++++ ctdb/tests/tool/stubby.natgwlist.009.sh | 36 +++++++++++++++ ctdb/tests/tool/stubby.natgwlist.010.sh | 37 +++++++++++++++ ctdb/tools/ctdb.c | 80 +++++++++++++++++++-------------- 4 files changed, 143 insertions(+), 33 deletions(-) create mode 100755 ctdb/tests/tool/stubby.natgwlist.009.sh create mode 100755 ctdb/tests/tool/stubby.natgwlist.010.sh diff --git a/ctdb/tests/src/ctdb_test_stubs.c b/ctdb/tests/src/ctdb_test_stubs.c index 6149b3d334a..d36fe815678 100644 --- a/ctdb/tests/src/ctdb_test_stubs.c +++ b/ctdb/tests/src/ctdb_test_stubs.c @@ -30,6 +30,10 @@ static struct ctdb_context *ctdb_global; * -CTDB_CAP_RECMASTER. LVS can be faked on by adding * CTDB_CAP_LVS. */ + +/* A fake flag that is only supported by some functions */ +#define NODE_FLAGS_FAKE_TIMEOUT 0x80000000 + void ctdb_test_stubs_read_nodemap(struct ctdb_context *ctdb) { char line[1024]; @@ -95,6 +99,12 @@ void ctdb_test_stubs_read_nodemap(struct ctdb_context *ctdb) capabilities &= ~CTDB_CAP_NATGW; } else if (strcmp(tok, "CTDB_CAP_LVS") == 0) { capabilities |= CTDB_CAP_LVS; + } else if (strcmp(tok, "TIMEOUT") == 0) { + /* This can be done with just a flag + * value but it is probably clearer + * and less error-prone to fake this + * with an explicit token */ + flags |= NODE_FLAGS_FAKE_TIMEOUT; } tok = strtok(NULL, " \t"); } @@ -550,6 +560,19 @@ int ctdb_ctrl_getcapabilities_stub(struct ctdb_context *ctdb, struct timeval timeout, uint32_t destnode, uint32_t *capabilities) { + + if (ctdb->nodes[destnode]->flags & NODE_FLAGS_FAKE_TIMEOUT) { + /* Placeholder for line#, instead of __location__ */ + DEBUG(DEBUG_ERR, + ("__LOCATION__ control timed out." + " reqid:1234567890 opcode:80 dstnode:%d\n", destnode)); + DEBUG(DEBUG_ERR, + ("__LOCATION__ ctdb_control_recv failed\n")); + DEBUG(DEBUG_ERR, + ("__LOCATION__ ctdb_ctrl_getcapabilities_recv failed\n")); + return -1; + } + if (ctdb->nodes[destnode]->flags & NODE_FLAGS_DISCONNECTED) { DEBUG(DEBUG_ERR, ("ctdb_control error: 'ctdb_control to disconnected node\n")); diff --git a/ctdb/tests/tool/stubby.natgwlist.009.sh b/ctdb/tests/tool/stubby.natgwlist.009.sh new file mode 100755 index 00000000000..7534fd220ac --- /dev/null +++ b/ctdb/tests/tool/stubby.natgwlist.009.sh @@ -0,0 +1,36 @@ +#!/bin/sh + +. "${TEST_SCRIPTS_DIR}/unit.sh" + +define_test "3 nodes, all in natgw group, 1 actual time-out" + +setup_natgw <nodes[ret->num] = nodemap->nodes[i]; ret->num++; + if (first_only) { + break; + } } return ret; @@ -1252,7 +1256,7 @@ static int control_natgwlist(struct ctdb_context *ctdb, int argc, const char **a int i, ret; struct pnn_node *natgw_nodes = NULL; struct ctdb_node_map *orig_nodemap=NULL; - struct ctdb_node_map *cnodemap, *nodemap; + struct ctdb_node_map *nodemap; uint32_t mypnn, pnn; const char *ip; @@ -1293,21 +1297,15 @@ static int control_natgwlist(struct ctdb_context *ctdb, int argc, const char **a goto done; } - /* Get a nodemap that includes only the nodes with the NATGW - * capability */ - cnodemap = filter_nodemap_by_capabilities(ctdb, nodemap, - CTDB_CAP_NATGW); - if (cnodemap == NULL) { - ret = -1; - goto done; - } - ret = 2; /* matches ENOENT */ pnn = -1; ip = "0.0.0.0"; + /* For each flag mask... */ for (i = 0; exclude_flags[i] != 0; i++) { + /* ... get a nodemap that excludes nodes with with + * masked flags... */ struct ctdb_node_map *t = - filter_nodemap_by_flags(ctdb, cnodemap, + filter_nodemap_by_flags(ctdb, nodemap, exclude_flags[i]); if (t == NULL) { /* No memory */ @@ -1315,10 +1313,23 @@ static int control_natgwlist(struct ctdb_context *ctdb, int argc, const char **a goto done; } if (t->num > 0) { - ret = 0; - pnn = t->nodes[0].pnn; - ip = ctdb_addr_to_str(&t->nodes[0].addr); - break; + /* ... and find the first node with the NATGW + * capability */ + struct ctdb_node_map *n; + n = filter_nodemap_by_capabilities(ctdb, t, + CTDB_CAP_NATGW, + true); + if (n == NULL) { + /* No memory */ + ret = -1; + goto done; + } + if (n->num > 0) { + ret = 0; + pnn = n->nodes[0].pnn; + ip = ctdb_addr_to_str(&n->nodes[0].addr); + break; + } } talloc_free(t); } @@ -3569,7 +3580,7 @@ static int control_lvs(struct ctdb_context *ctdb, int argc, const char **argv) } nodemap = filter_nodemap_by_capabilities(ctdb, orig_nodemap, - CTDB_CAP_LVS); + CTDB_CAP_LVS, false); if (nodemap == NULL) { /* No memory */ ret = -1; @@ -3609,26 +3620,17 @@ done: static int control_lvsmaster(struct ctdb_context *ctdb, int argc, const char **argv) { TALLOC_CTX *tmp_ctx = talloc_new(ctdb); - struct ctdb_node_map *orig_nodemap=NULL; - struct ctdb_node_map *nodemap; + struct ctdb_node_map *nodemap=NULL; int i, ret; ret = ctdb_ctrl_getnodemap(ctdb, TIMELIMIT(), options.pnn, - tmp_ctx, &orig_nodemap); + tmp_ctx, &nodemap); if (ret != 0) { DEBUG(DEBUG_ERR, ("Unable to get nodemap from node %u\n", options.pnn)); talloc_free(tmp_ctx); return -1; } - nodemap = filter_nodemap_by_capabilities(ctdb, orig_nodemap, - CTDB_CAP_LVS); - if (nodemap == NULL) { - /* No memory */ - ret = -1; - goto done; - } - for (i = 0; lvs_exclude_flags[i] != 0; i++) { struct ctdb_node_map *t = filter_nodemap_by_flags(ctdb, nodemap, @@ -3639,11 +3641,23 @@ static int control_lvsmaster(struct ctdb_context *ctdb, int argc, const char **a goto done; } if (t->num > 0) { - ret = 0; - printf(options.machinereadable ? - "%d\n" : "Node %d is LVS master\n", - t->nodes[0].pnn); - goto done; + struct ctdb_node_map *n; + n = filter_nodemap_by_capabilities(ctdb, + t, + CTDB_CAP_LVS, + true); + if (n == NULL) { + /* No memory */ + ret = -1; + goto done; + } + if (n->num > 0) { + ret = 0; + printf(options.machinereadable ? + "%d\n" : "Node %d is LVS master\n", + n->nodes[0].pnn); + goto done; + } } talloc_free(t); } -- 2.11.4.GIT