HDDS-11128. Wait for replication to settle instead of a transient replica count - #11010
Draft
smengcl wants to merge 3 commits into
Draft
HDDS-11128. Wait for replication to settle instead of a transient replica count#11010smengcl wants to merge 3 commits into
smengcl wants to merge 3 commits into
Conversation
… waits in TestReconAndAdminContainerCLI testNodesInDecommissionOrMaintenance intermittently times out at OzoneTestHelper.waitForReplicaCount while waiting for a decommission- triggered replica copy (3 -> 4 for the first node, 4 -> 5 for the second) to be reflected in SCM. The shared waitForReplicaCount helper used a fixed 30s budget for all 14 callers; HDDS-10582 only reduced its poll interval (1000ms -> 200ms) and kept the 30s total, so under a loaded CI runner the replica copy is not always observed in time and the test flakes. Add a 4-arg waitForReplicaCount overload that accepts a timeout, leaving the existing 3-arg method delegating with the same 30s default (no behavior change for the other callers). The decommission/maintenance replica-copy waits in TestReconAndAdminContainerCLI now use a 120s budget, matching the larger timeouts adopted elsewhere for the same class of replication waits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces flakiness in Recon/admin container CLI integration coverage by allowing callers of the shared OzoneTestHelper.waitForReplicaCount helper to specify a larger timeout, and then applying that higher timeout to the decommission/maintenance replica-copy waits that intermittently time out in CI.
Changes:
- Added a 4-argument
waitForReplicaCount(..., timeoutMillis)overload inOzoneTestHelper, keeping the existing 3-arg method as a 30s-default wrapper. - Updated
TestReconAndAdminContainerCLI#testNodesInDecommissionOrMaintenanceto use a 120s timeout when waiting for decommission/maintenance-triggered replica copies to be reflected in SCM.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/OzoneTestHelper.java | Introduces a timeout-parameter overload for waitForReplicaCount, preserving the original default behavior. |
| hadoop-ozone/integration-test-recon/src/test/java/org/apache/hadoop/ozone/recon/TestReconAndAdminContainerCLI.java | Uses a larger, named timeout constant for the decommission/maintenance replica-copy waits to reduce test flakiness. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ReplicaCount Javadoc link The happy path returns as soon as the copy lands, so the timeout only matters on a genuine failure; 120s was more than needed and, because this @flaky test is rerun on failure in the flaky split, an oversized budget multiplies wasted CI time on a real break. 60s (double the previous 30s) gives comfortable headroom for the tail latency while keeping the failing path bounded. Also qualify the OzoneTestHelper#waitForReplicaCount(long, int, MiniOzoneCluster) Javadoc link, which became ambiguous once the timeout overload was added (flagged by review, avoids doclint resolution warnings). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lica count testNodesInDecommissionOrMaintenance waited on a bare replica-count equality (countReplicas(...) == N) after each decommission/maintenance step. During decommission SCM is actively adding (and re-evaluating) replicas, so the count passes through the expected value transiently; a fixed-value poll can sample the wrong instant and time out. HDDS-10582 only shrank the poll interval (1000ms -> 200ms) to narrow that window, so the flake recurred (HDDS-11128). Add OzoneTestHelper.waitForStableReplicaCount, which returns only once replication has quiesced (ReplicationManager has no pending add/delete ops for the container) AND the count equals N, so the assertion is on a settled state rather than a transient one. It uses the same 30s budget as waitForReplicaCount; the elevated timeout is no longer needed because the call runs right after the DECOMMISSIONED/IN_MAINTENANCE gate, which already requires the new replica to exist, so the settle returns almost immediately. The existing waitForReplicaCount is left untouched for its other callers. The test remains @flaky("HDDS-11128") because no wait can prove non-flakiness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
smengcl
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/OzoneTestHelper.java:482
- An empty pending-op list is only a snapshot; it does not mean ReplicationManager has evaluated the latest node/replica state. This test configures RM to scan every 1 second, while this predicate polls every 200 ms, so it can accept an already-present expected count before the next scan schedules an add/delete and reproduce the same transient-state race.
ReplicationManager.processAll()is public specifically for tests and synchronized with its monitor thread; run a pass before checking the pending operations so the observed emptiness reflects the current state.
GenericTestUtils.waitFor(() ->
replicationManager.getPendingReplicationOps(cid).isEmpty() && countReplicas(containerID, cluster) == count,
200, 30000);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generated-by: Claude Code (Opus 4.8)
What changes were proposed in this pull request?
TestReconAndAdminContainerCLI#testNodesInDecommissionOrMaintenanceintermittently timed out atOzoneTestHelper.waitForReplicaCountafter a decommission/maintenance step (HDDS-11128).Root cause: the test waited on a bare replica-count equality (
countReplicas(...) == N). During decommission/maintenance SCM is actively adding and re-evaluating replicas, so the count passes through the expected value transiently, and a fixed-value poll can sample the wrong instant and time out. HDDS-10582 previously only shrank the poll interval (1000ms to 200ms) to narrow that window, so the flake recurred.This adds
OzoneTestHelper.waitForStableReplicaCount, which returns only once replication has quiesced (ReplicationManagerreports no pending add/delete ops for the container) AND the count equals N, so the assertion is on a settled state rather than a transient one. It uses the same 30s budget aswaitForReplicaCount; no larger timeout is needed, because the call runs right after theDECOMMISSIONED/IN_MAINTENANCEgate, which already requires the new replica to exist, so the settle returns almost immediately. The existingwaitForReplicaCountand its other callers are left untouched.The test stays tagged
@Flaky("HDDS-11128")because no wait can prove non-flakiness.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11128
How was this patch tested?
Existing test, no new coverage added (synchronization-only change to a flaky integration test).
mvn -pl :ozone-integration-test test-compileandmvn -pl :ozone-integration-test-recon test-compile: both compile.checkstyle.shon both changed modules: no violations.The fix is in the wait predicate (settle on no pending replication ops) rather than the timeout, which addresses the transient-count race directly. As with any flaky fix, a single green run cannot prove non-flakiness, so the
@Flakytag stays.