smengcl commented on code in PR #11506:
URL: https://github.com/apache/ozone/pull/11506#discussion_r4256012902
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestFailureHandlingByClient.java:
##########
@@ -159,8 +162,28 @@ public void restartDownDataNodes() throws Exception {
for (DatanodeDetails dataNode : restartDataNodes) {
cluster.restartHddsDatanode(dataNode, false);
}
- restartDataNodes.clear();
cluster.waitForClusterToBeReady();
+ // A restarted datanode can take tens of seconds to rejoin its Ratis
groups, while SCM keeps allocating
+ // blocks on those pipelines. Wait for every group on it to have a leader
before the next test writes.
+ for (DatanodeDetails dataNode : restartDataNodes) {
+ RaftServer server = assertInstanceOf(XceiverServerRatis.class,
+
cluster.getHddsDatanode(dataNode).getDatanodeStateMachine().getContainer().getWriteChannel()).getServer();
+ GenericTestUtils.waitFor(() -> allGroupsHaveLeader(server), 500,
120_000);
+ }
+ restartDataNodes.clear();
+ }
+
+ private static boolean allGroupsHaveLeader(RaftServer server) {
+ try {
+ for (RaftGroupId groupId : server.getGroupIds()) {
+ if (server.getDivision(groupId).getInfo().getLeaderId() == null) {
Review Comment:
No change: in Ratis 3.3.1 `RaftServerProxy.getDivision` never returns null;
a missing or removed group completes with `GroupMismatchException` (an
`IOException`), which the helper already catches and treats as not ready.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestFailureHandlingByClient.java:
##########
@@ -159,8 +162,28 @@ public void restartDownDataNodes() throws Exception {
for (DatanodeDetails dataNode : restartDataNodes) {
cluster.restartHddsDatanode(dataNode, false);
}
- restartDataNodes.clear();
cluster.waitForClusterToBeReady();
+ // A restarted datanode can take tens of seconds to rejoin its Ratis
groups, while SCM keeps allocating
+ // blocks on those pipelines. Wait for every group on it to have a leader
before the next test writes.
+ for (DatanodeDetails dataNode : restartDataNodes) {
+ RaftServer server = assertInstanceOf(XceiverServerRatis.class,
+
cluster.getHddsDatanode(dataNode).getDatanodeStateMachine().getContainer().getWriteChannel()).getServer();
+ GenericTestUtils.waitFor(() -> allGroupsHaveLeader(server), 500,
120_000);
+ }
+ restartDataNodes.clear();
+ }
+
+ private static boolean allGroupsHaveLeader(RaftServer server) {
+ try {
+ for (RaftGroupId groupId : server.getGroupIds()) {
+ if (server.getDivision(groupId).getInfo().getLeaderId() == null) {
+ return false;
+ }
+ }
+ return true;
+ } catch (IOException e) {
+ return false;
+ }
}
Review Comment:
No change: Ratis loads the group directories synchronously in `initGroups`
when the server is built, before the datanode registers with SCM, so an empty
set means the datanode has no groups. Treating that as not ready would make the
wait hang for a datanode whose pipelines were all removed.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestFailureHandlingByClient.java:
##########
@@ -159,8 +162,28 @@ public void restartDownDataNodes() throws Exception {
for (DatanodeDetails dataNode : restartDataNodes) {
cluster.restartHddsDatanode(dataNode, false);
}
- restartDataNodes.clear();
cluster.waitForClusterToBeReady();
+ // A restarted datanode can take tens of seconds to rejoin its Ratis
groups, while SCM keeps allocating
+ // blocks on those pipelines. Wait for every group on it to have a leader
before the next test writes.
+ for (DatanodeDetails dataNode : restartDataNodes) {
+ RaftServer server = assertInstanceOf(XceiverServerRatis.class,
+
cluster.getHddsDatanode(dataNode).getDatanodeStateMachine().getContainer().getWriteChannel()).getServer();
+ GenericTestUtils.waitFor(() -> allGroupsHaveLeader(server), 500,
120_000);
Review Comment:
No change: the rest of this class passes `waitFor` intervals and timeouts
inline (e.g. `waitFor(hddsDatanode::isStopped, 1000, 30000)`), so this follows
the local style.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
