Skip to content

Reduce log noise for k8s NodeRoleWatcher #19077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
capistrant merged 1 commit into from
Mar 4, 2026
Merged

Reduce log noise for k8s NodeRoleWatcher #19077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions server/src/main/java/org/apache/druid/discovery/BaseNodeRoleWatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ public void childAdded(DiscoveryDruidNode druidNode)
return;
}

LOGGER.info("Node [%s] of role [%s] detected.", druidNode.getDruidNode().getUriToUse(), nodeRole.getJsonName());

addNode(druidNode);
}
}
Expand All @@ -152,6 +150,7 @@ private void addNode(DiscoveryDruidNode druidNode)
{
DiscoveryDruidNode prev = nodes.putIfAbsent(druidNode.getDruidNode().getHostAndPortToUse(), druidNode);
if (prev == null) {
LOGGER.info("Node [%s] of role [%s] detected.", druidNode.getDruidNode().getUriToUse(), nodeRole.getJsonName());
// No need to wait on CountDownLatch, because we are holding the lock under which it could only be counted down.
if (cacheInitialized.getCount() == 0) {
List<DiscoveryDruidNode> newNode = ImmutableList.of(druidNode);
Expand All @@ -165,7 +164,7 @@ private void addNode(DiscoveryDruidNode druidNode)
}
}
} else {
LOGGER.error(
LOGGER.debug(
"Node [%s] of role [%s] discovered but existed already [%s].",
druidNode.getDruidNode().getUriToUse(),
nodeRole.getJsonName(),
Expand Down
41 changes: 41 additions & 0 deletions server/src/test/java/org/apache/druid/discovery/BaseNodeRoleWatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,47 @@ public void testGetAllNodesBeforeTimeout() throws InterruptedException
assertListener(listener1, true, ImmutableList.of(broker1, broker3), ImmutableList.of());
}

@Test(timeout = 60_000L)
public void testDuplicateChildAddedAfterResetNodesDoesNotNotifyListeners()
{
BaseNodeRoleWatcher nodeRoleWatcher = BaseNodeRoleWatcher.create(exec, NodeRole.BROKER);

DiscoveryDruidNode broker1 = buildDiscoveryDruidNode(NodeRole.BROKER, "broker1");
DiscoveryDruidNode broker2 = buildDiscoveryDruidNode(NodeRole.BROKER, "broker2");

// Initial discovery and cache initialization
nodeRoleWatcher.childAdded(broker1);
nodeRoleWatcher.childAdded(broker2);
nodeRoleWatcher.cacheInitialized();

TestListener listener = new TestListener();
nodeRoleWatcher.registerListener(listener);

// Verify listener received the initial nodes
Assert.assertEquals(ImmutableList.of(broker1, broker2), listener.nodesAddedList);

// Simulate watch reconnect: resetNodes with the same set of nodes
LinkedHashMap<String, DiscoveryDruidNode> resetMap = new LinkedHashMap<>();
resetMap.put(broker1.getDruidNode().getHostAndPortToUse(), broker1);
resetMap.put(broker2.getDruidNode().getHostAndPortToUse(), broker2);
nodeRoleWatcher.resetNodes(resetMap);

// No new additions or removals since the node set is unchanged
Assert.assertEquals(ImmutableList.of(broker1, broker2), listener.nodesAddedList);
Assert.assertTrue(listener.nodesRemovedList.isEmpty());

// Simulate K8s watch replaying ADDED events for already-present nodes
nodeRoleWatcher.childAdded(broker1);
nodeRoleWatcher.childAdded(broker2);

// Listeners should NOT be notified again — the duplicate adds are no-ops
Assert.assertEquals(ImmutableList.of(broker1, broker2), listener.nodesAddedList);
Assert.assertTrue(listener.nodesRemovedList.isEmpty());

// The nodes map should still contain exactly the same two nodes
Assert.assertEquals(ImmutableSet.of(broker1, broker2), new HashSet<>(nodeRoleWatcher.getAllNodes()));
}

private DiscoveryDruidNode buildDiscoveryDruidNode(NodeRole role, String host)
{
return new DiscoveryDruidNode(
Expand Down
Loading