Skip to content

Commit b54720e

Browse files
authored
dynamic_modules: skip host-set republish when addHosts adds no new hosts (#45627)
**Commit Message:** dynamic_modules: skip host-set republish when addHosts adds no new hosts **Additional Description:** A dynamic module cluster re-applies its full host set from the control plane periodically. addHosts already skips addresses already present in the cluster, but when a batch contains only such addresses (eg., a control plane re-send on reconnect), it still rebuilds and republishes the unchanged host set: it copies the whole host vector, re-partitions every host, rebuilds per-locality, and calls updateHosts, which fans out to every worker and fires a (0, 0) membership update. In a cluster with millions of hosts that is a full O(N) rebuild and cross-thread publish for zero membership change. This returns early from addHosts when no new hosts remain after filtering, so a no-op re-send does no per-worker republish. Risk Level: Low Testing: Unit Test Docs Changes: N.A Release Notes: N.A Platform Specific Features: N.A Signed-off-by: Basundhara Chakrabarty <basundhara17061996@gmail.com>
1 parent 214a094 commit b54720e

2 files changed

Lines changed: 24 additions & 1 deletion

File tree

source/extensions/clusters/dynamic_modules/cluster.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,13 @@ bool DynamicModuleCluster::addHosts(
453453
result_hosts.emplace_back(std::move(host_result.value()));
454454
}
455455

456+
// Every incoming address was already present in the host set, so there is nothing to add. Return
457+
// without rebuilding and republishing the unchanged host set, which would otherwise fan out a
458+
// no-op membership update to every worker.
459+
if (result_hosts.empty()) {
460+
return true;
461+
}
462+
456463
{
457464
absl::WriterMutexLock lock(host_map_lock_);
458465
for (const auto& host : result_hosts) {

test/extensions/clusters/dynamic_modules/cluster_test.cc

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,12 +337,28 @@ TEST_F(DynamicModuleClusterTest, DuplicateHostDetection) {
337337
EXPECT_EQ(3, DynamicModuleClusterTestPeer::getHostMapSize(*cluster));
338338
EXPECT_EQ(3, cluster->prioritySet().hostSetsPerPriority()[0]->hosts().size());
339339

340-
// A batch of only existing addresses adds nothing but still succeeds.
340+
// Capture the host-set partition identity before a no-op push. A rebuild would replace these
341+
// shared_ptrs (partitionHosts allocates new vectors), so identity is a precise "did we rebuild"
342+
// signal. Also subscribe a membership callback: a republish fires it even with a (0,0) diff.
343+
const auto* hosts_ptr_before = cluster->prioritySet().hostSetsPerPriority()[0]->hostsPtr().get();
344+
const auto* healthy_ptr_before =
345+
cluster->prioritySet().hostSetsPerPriority()[0]->healthyHostsPtr().get();
346+
int membership_updates = 0;
347+
auto cb_handle = cluster->prioritySet().addMemberUpdateCb(
348+
[&](const Upstream::HostVector&, const Upstream::HostVector&) { ++membership_updates; });
349+
350+
// A batch of only existing addresses adds nothing but still succeeds, and must not rebuild or
351+
// republish the host set.
341352
std::vector<Upstream::HostSharedPtr> hosts3;
342353
ASSERT_TRUE(addSimpleHosts(*cluster, {"127.0.0.1:10001", "127.0.0.1:10003"}, {1, 1}, hosts3));
343354
EXPECT_EQ(0, hosts3.size());
344355
EXPECT_EQ(3, DynamicModuleClusterTestPeer::getHostMapSize(*cluster));
345356
EXPECT_EQ(3, cluster->prioritySet().hostSetsPerPriority()[0]->hosts().size());
357+
// No rebuild: the partition shared_ptrs are unchanged and no membership callback fired.
358+
EXPECT_EQ(hosts_ptr_before, cluster->prioritySet().hostSetsPerPriority()[0]->hostsPtr().get());
359+
EXPECT_EQ(healthy_ptr_before,
360+
cluster->prioritySet().hostSetsPerPriority()[0]->healthyHostsPtr().get());
361+
EXPECT_EQ(0, membership_updates);
346362
}
347363

348364
// Test that invalid addresses cause the entire batch to fail.

0 commit comments

Comments
 (0)