Skip to content

HDDS-16224. Wait for a replica with the container and block before reading in TestFailureHandlingByClient - #11065

Open
rich7420 wants to merge 2 commits into
apache:masterfrom
rich7420:HDDS-16224
Open

HDDS-16224. Wait for a replica with the container and block before reading in TestFailureHandlingByClient#11065
rich7420 wants to merge 2 commits into
apache:masterfrom
rich7420:HDDS-16224

Conversation

@rich7420

@rich7420 rich7420 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

TestFailureHandlingByClient.testBlockWritesWithDnFailures intermittently failed while reading a block back from a datanode container replica.

Failure. After the key is written and the pipeline is shut down, the test read the two blocks' KeyValueContainerData from a fixed datanode (a specific pipeline index). With RATIS/THREE and majority commit, that fixed node can be a lagging follower that has not yet applied the container (or has the container but not the target block), so:

  • getContainer(containerId) returned null → NPE, or
  • the block was absent / had fewer chunks than expected → NPE on blockData.getChunks() or a chunk-count assertion mismatch.

Fix. Read from a replica that has both the container and the target block applied, polling all datanodes with GenericTestUtils.waitFor until one is ready. This removes the dependency on a particular replica's apply progress, which is the source of the flakiness. No production code changes; test-only.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16224

How was this patch tested?

…andlingByClient

testBlockCountOnFailures read each block's container from a fixed pipeline index
(block1DNs.get(2) / block2DNs.get(0)). The second block is written to a fresh
pipeline allocated after the injected DN failure and commits on a majority of its
replicas, so the indexed node may be the lagging replica that has not yet applied
the create-container transaction: getContainer(id) returns null and the cast to
KeyValueContainer NPEs (testBlockWritesWithDnFailures:220 ->
testBlockCountOnFailures:297). Use ClusterContainersUtil.getContainerByID, which
returns the container from whichever datanode already has it, instead of a fixed
pipeline index.
Copilot AI lite review requested due to automatic review settings August 19, 2026 14:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rich7420

Copy link
Copy Markdown
Contributor Author

cc @ChenSammi
would you like to take a look?

@chihsuan chihsuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @rich7420. Overall, the change looks good. I noticed that the PR title mentions waiting, while the patch searches for a replica where the container is already available.

I suggest using GenericTestUtils.waitFor until both the container and target block are available on the same replica to match PR title, or simplifying the title to something like “Fix intermittent failure in TestFailureHandlingByClient.”

It would also be helpful if the PR description explained the failure and how this change fixes it. Thanks!

…r and block

Read from any replica that has both the container and the target block applied
(polling), instead of a fixed replica or one chosen only for having the
container. A lagging Ratis follower can hold the container without the block,
which is the source of the intermittent failure.
@rich7420 rich7420 changed the title HDDS-16224. Wait for the container to be applied before reading it in TestFailureHandlingByClient HDDS-16224. Wait for a replica with the container and block before reading in TestFailureHandlingByClient Aug 22, 2026
@rich7420

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants