Repository navigation
Conversation
…File Container import no longer generates SST files, so the test only asserts that none exist. Create 10 containers per volume instead of 200 and drop the 7s wait for a compaction that has no input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Advance a MockClock past the retry delay instead of sleeping 2.1s between attempts, as TestReconTaskControllerImpl already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use a zero read retry interval instead of the default 1s, except in testRefreshPipelineFunction which asserts the retry delay log. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The handler slept 2s for each of 11 events on a single thread. 500ms still leaves events pending when the test checks progress after 500ms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
1 open finding
What changed in this PR
This test-only PR reduces execution time by replacing real waits with virtual time and shortening unnecessary test workloads.
Changes:
- Uses
MockClockfor Recon retry tests. - Reduces event-handler sleep and container test data volume.
- Sets block read retry intervals to zero where delay is not under test.
| File | Description |
|---|---|
| hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestEventBufferOverflow.java | Updated as part of this pull request. |
| hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/events/TestEventQueue.java | Updated as part of this pull request. |
| hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java | Updated as part of this pull request. |
| hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/storage/TestBlockInputStream.java | Updated as part of this pull request. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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 5.5)
What changes were proposed in this pull request?
Four unit test classes spend most of their time on waits or test data that no assertion depends on. Test only change, one commit per class.
TestKeyValueContainer#testAutoCompactionSmallSstFilecreates 400 containers with 500 keys each and then sleeps 7s. Since HDDS-12233 container import no longer generates SST files, so the test only asserts that none exist. Create 10 containers per volume and drop the sleep.TestEventBufferOverflowsleeps 2.1s between retry attempts. Advance aMockClockpast the retry delay instead, asTestReconTaskControllerImplalready does.TestBlockInputStreamwaits the default 1sozone.client.read.retry.intervalbefore each retry. Set the interval to 0, except intestRefreshPipelineFunction, which asserts the retry delay log.TestEventQueueuses a handler that sleeps 2s for each of 11 events, processed by a single thread. Sleep 500ms instead. The test checks after 500ms that between 1 and 10 events are scheduled, and the 11th is scheduled only after about 5s.Test class time in CI, four recent master runs compared to a run of these changes on my fork:
TestKeyValueContainerTestEventBufferOverflowTestBlockInputStreamTestEventQueueWhat is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16933
How was this patch tested?
🤖 Generated with Claude Code