abhishekagarwal87 commented on code in PR #15726:
URL: https://github.com/apache/druid/pull/15726#discussion_r1512341308
##########
server/src/test/java/org/apache/druid/discovery/BaseNodeRoleWatcherTest.java:
##########
@@ -116,6 +117,70 @@ public void testGeneralUseSimulation()
assertListener(listener3, true, nodesAdded, nodesRemoved);
}
+ @Test(timeout = 60_000L)
+ public void testRegisterListenerBeforeTimeout() throws InterruptedException
+ {
+ BaseNodeRoleWatcher nodeRoleWatcher = new BaseNodeRoleWatcher(exec,
NodeRole.BROKER, 3);
+
+ TestListener listener1 = new TestListener();
+ nodeRoleWatcher.registerListener(listener1);
+
+ DiscoveryDruidNode broker1 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker1");
+ DiscoveryDruidNode broker2 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker2");
+ DiscoveryDruidNode broker3 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker3");
+
+ DiscoveryDruidNode notBroker = new DiscoveryDruidNode(
+ new DruidNode("s3", "h3", false, 8080, null, true, false),
+ NodeRole.COORDINATOR,
+ ImmutableMap.of()
+ );
+
+ nodeRoleWatcher.childAdded(broker1);
+ nodeRoleWatcher.childAdded(notBroker);
+ nodeRoleWatcher.childAdded(broker3);
+ nodeRoleWatcher.childRemoved(broker2);
+
+ assertListener(listener1, false, Collections.emptyList(),
Collections.emptyList());
+
+ Thread.sleep(3100);
Review Comment:
With this approach, the 3 seconds could be reduced to 1 second.
##########
server/src/test/java/org/apache/druid/discovery/BaseNodeRoleWatcherTest.java:
##########
@@ -116,6 +117,70 @@ public void testGeneralUseSimulation()
assertListener(listener3, true, nodesAdded, nodesRemoved);
}
+ @Test(timeout = 60_000L)
+ public void testRegisterListenerBeforeTimeout() throws InterruptedException
+ {
+ BaseNodeRoleWatcher nodeRoleWatcher = new BaseNodeRoleWatcher(exec,
NodeRole.BROKER, 3);
+
+ TestListener listener1 = new TestListener();
+ nodeRoleWatcher.registerListener(listener1);
+
+ DiscoveryDruidNode broker1 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker1");
+ DiscoveryDruidNode broker2 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker2");
+ DiscoveryDruidNode broker3 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker3");
+
+ DiscoveryDruidNode notBroker = new DiscoveryDruidNode(
+ new DruidNode("s3", "h3", false, 8080, null, true, false),
+ NodeRole.COORDINATOR,
+ ImmutableMap.of()
+ );
+
+ nodeRoleWatcher.childAdded(broker1);
+ nodeRoleWatcher.childAdded(notBroker);
+ nodeRoleWatcher.childAdded(broker3);
+ nodeRoleWatcher.childRemoved(broker2);
+
+ assertListener(listener1, false, Collections.emptyList(),
Collections.emptyList());
+
+ Thread.sleep(3100);
Review Comment:
You should also shutdown all the executors in a tear method.
##########
server/src/test/java/org/apache/druid/discovery/BaseNodeRoleWatcherTest.java:
##########
@@ -116,6 +117,70 @@ public void testGeneralUseSimulation()
assertListener(listener3, true, nodesAdded, nodesRemoved);
}
+ @Test(timeout = 60_000L)
+ public void testRegisterListenerBeforeTimeout() throws InterruptedException
+ {
+ BaseNodeRoleWatcher nodeRoleWatcher = new BaseNodeRoleWatcher(exec,
NodeRole.BROKER, 3);
+
+ TestListener listener1 = new TestListener();
+ nodeRoleWatcher.registerListener(listener1);
+
+ DiscoveryDruidNode broker1 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker1");
+ DiscoveryDruidNode broker2 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker2");
+ DiscoveryDruidNode broker3 = buildDiscoveryDruidNode(NodeRole.BROKER,
"broker3");
+
+ DiscoveryDruidNode notBroker = new DiscoveryDruidNode(
+ new DruidNode("s3", "h3", false, 8080, null, true, false),
+ NodeRole.COORDINATOR,
+ ImmutableMap.of()
+ );
+
+ nodeRoleWatcher.childAdded(broker1);
+ nodeRoleWatcher.childAdded(notBroker);
+ nodeRoleWatcher.childAdded(broker3);
+ nodeRoleWatcher.childRemoved(broker2);
+
+ assertListener(listener1, false, Collections.emptyList(),
Collections.emptyList());
+
+ Thread.sleep(3100);
Review Comment:
Actually, that's not a good idea since that can potentially stall the test.
So either a latch-based approach still might be better or we call
`getAllNodes()` in a way that we can interrupt the thread.
So in test, we can do
Future future = anotherThreadPool.submit(listener.getAllNodes())
future.wait(some-timeout-more-than-3-seconds)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
