From 9d6f922c788a0a1c4d95b3b45427aa5b6ea46ee7 Mon Sep 17 00:00:00 2001 From: "wangwenhai.26" Date: Mon, 17 Aug 2026 16:49:38 +0800 Subject: [PATCH] ZOOKEEPER-2466: Fix C client skipping servers when reconnecting --- .../zookeeper-client-c/include/zookeeper.h | 6 +++--- .../zookeeper-client-c/src/zookeeper.c | 13 +++++++------ .../zookeeper-client-c/tests/TestClient.cc | 19 +++++++++++++++++-- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/zookeeper-client/zookeeper-client-c/include/zookeeper.h b/zookeeper-client/zookeeper-client-c/include/zookeeper.h index ead328a94a0..d8f60e1e344 100644 --- a/zookeeper-client/zookeeper-client-c/include/zookeeper.h +++ b/zookeeper-client/zookeeper-client-c/include/zookeeper.h @@ -843,9 +843,9 @@ ZOOAPI int zoo_set_servers_resolution_delay(zhandle_t *zh, int delay_ms); * * This method allows a client to cycle through the list of servers in it's * connection pool to be used on the next connection attempt. This function does - * not actually trigger a connection or state change in any way. Its purpose is - * to allow testing changing servers on the fly and the probabilistic load - * balancing algorithm. + * not actually trigger a connection, but it updates the handle with the address + * of the next server to connect to. Its purpose is to allow testing changing + * servers on the fly and the probabilistic load balancing algorithm. */ ZOOAPI void zoo_cycle_next_server(zhandle_t *zh); diff --git a/zookeeper-client/zookeeper-client-c/src/zookeeper.c b/zookeeper-client/zookeeper-client-c/src/zookeeper.c index cdd9d74a537..c260ec083a0 100644 --- a/zookeeper-client/zookeeper-client-c/src/zookeeper.c +++ b/zookeeper-client/zookeeper-client-c/src/zookeeper.c @@ -1604,6 +1604,8 @@ void zoo_cycle_next_server(zhandle_t *zh) zh->reconfig = 0; } + // Delay the next attempt when this selection wraps the server list. + zh->delay = addrvec_atend(&zh->addrs); addrvec_next(&zh->addrs, &zh->addr_cur); unlock_reconfig(zh); @@ -2030,11 +2032,6 @@ static void cleanup(zhandle_t *zh,int rc) static void handle_error(zhandle_t *zh,int rc) { cleanup(zh, rc); - // NOTE: If we're at the end of the list of addresses to connect to, then - // we want to delay the next connection attempt to avoid spinning. - // Then increment what host we'll connect to since we failed to connect to current - zh->delay = addrvec_atend(&zh->addrs); - addrvec_next(&zh->addrs, &zh->addr_cur); } static int handle_socket_error_msg(zhandle_t *zh, int line, const char *func, int rc, @@ -2694,7 +2691,11 @@ int zookeeper_interest(zhandle_t *zh, socket_t *fd, int *interest, format_endpoint_info(&addr)); cleanup(zh, ZOK); } else { - addrvec_next(&zh->addrs, NULL); + lock_reconfig(zh); + if (!zh->reconfig) { + addrvec_next(&zh->addrs, NULL); + } + unlock_reconfig(zh); } } send_to = min(send_to, zh->ping_rw_timeout - idle_ping_rw); diff --git a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc b/zookeeper-client/zookeeper-client-c/tests/TestClient.cc index 83a7203d7b8..047b24416fa 100644 --- a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc +++ b/zookeeper-client/zookeeper-client-c/tests/TestClient.cc @@ -345,11 +345,26 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture /** ZOOKEEPER-1057 This checks that the client connects to the second server when the first is not reachable **/ void testFirstServerDown() { watchctx_t ctx; + const char *hosts = "127.0.0.1:22182,127.0.0.1:22181"; + const char *server = "127.0.0.1:22181"; + char cmd[1024]; zoo_deterministic_conn_order(true); - zhandle_t* zk = createClient("127.0.0.1:22182,127.0.0.1:22181", &ctx); - CPPUNIT_ASSERT(zk != 0); + // ZOOKEEPER-2466: exhaust the server list before making the second + // server available, so reconnecting must not skip it. + snprintf(cmd, sizeof(cmd), "%s stop %s", ZKSERVER_CMD, server); + CPPUNIT_ASSERT(system(cmd) == 0); + + zhandle_t* zk = createClient(hosts, &ctx); + bool clientCreated = zk != 0; + bool connectedWhileStopped = clientCreated && ctx.waitForConnected(zk); + + snprintf(cmd, sizeof(cmd), "%s startClean %s", ZKSERVER_CMD, server); + CPPUNIT_ASSERT(system(cmd) == 0); + + CPPUNIT_ASSERT(clientCreated); + CPPUNIT_ASSERT(!connectedWhileStopped); CPPUNIT_ASSERT(ctx.waitForConnected(zk)); }