smengcl commented on code in PR #10704:
URL: https://github.com/apache/ozone/pull/10704#discussion_r3562166574
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java:
##########
@@ -229,10 +229,9 @@ public void testScmHeartbeat()
nodeManager.processHeartbeat(datanodeDetails);
}
- //TODO: wait for heartbeat to be processed
- Thread.sleep(4 * 1000);
- assertEquals(nodeManager.getAllNodes().size(), registeredNodes,
- "Heartbeat thread should have picked up the scheduled heartbeats.");
+ // Heartbeat thread should pick up the scheduled heartbeats.
+ GenericTestUtils.waitFor(
+ () -> nodeManager.getAllNodes().size() == registeredNodes, 100,
4000);
Review Comment:
The original sleep+assertEquals had the same property: getAllNodes().size()
is satisfied at registration, so it never waited on heartbeat processing. This
conversion just preserves that behavior.
The suggested in-service healthy count won't fix it either, since node
health is set synchronously in newNodeStatus() at addNode time, not by
heartbeat processing. And processHeartbeat() itself runs synchronously in the
test loop, so there is nothing async to wait on here.
Updated the test to assert on the NumHBProcessed metric, which is the signal
that actually reflects processed heartbeats. Since processHeartbeat() runs
synchronously in the test loop, this is a plain assertion with no wait needed.
--
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]
