HDDS-16387. ContainerBalancer status --verbose throws IllegalStateException after a rejected start. - #11275
Open
navinko wants to merge 5 commits into
Open
HDDS-16387. ContainerBalancer status --verbose throws IllegalStateException after a rejected start. #11275navinko wants to merge 5 commits into
navinko wants to merge 5 commits into
Conversation
sreejasahithi
self-requested a review
September 20, 2026 17:01
sravani-revuri
left a comment
sravani-revuri
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the patch @navinko ! Few minor nits below.
| */ | ||
| @Test | ||
| public void testRejectedStartDoesNotModifyStartedAt() throws Exception { | ||
| // The test first starts and immediately stops the balancer normally.This creates a realistic "previous state" |
Contributor
There was a problem hiding this comment.
nit:
Suggested change
| // The test first starts and immediately stops the balancer normally.This creates a realistic "previous state" | |
| // The test first starts and immediately stops the balancer normally. This creates a realistic "previous state" |
| // The test first starts and immediately stops the balancer normally.This creates a realistic "previous state" | ||
| // in system memory and later trigger a bad request and verify that startedAt remains identical to startedAtBefore, | ||
| // rather than being wrongly overwritten with "now", which would make startedAt come after stoppedAt which resulted | ||
| // negative balancing duration. |
Contributor
There was a problem hiding this comment.
nit:
Suggested change
| // negative balancing duration. | |
| // in a negative balancing duration. |
Contributor
Author
There was a problem hiding this comment.
corrected the comment.
|
|
||
| // A start that is rejected during validation. Here we used an invalid config this is just one of many rejection | ||
| // paths (safe mode, non-leader SCM, already running, etc.). | ||
| OzoneConfiguration invalidConf = new OzoneConfiguration(); |
Contributor
There was a problem hiding this comment.
nit: consider building the invalid config similar to testRejectInvalidStartupConfiguration ( use setMoveReplicationTimeout / setMoveTimeout on balancerConfiguration ) instead of creating a new OzoneConfiguration .
Contributor
Author
There was a problem hiding this comment.
Thank you! Ah i missed reusing the existing pattern, updated now.
Contributor
Author
|
Thanks @sreejasahithi @sravani-revuri for the review . I tried addressing the review suggestions. |
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
2 participants
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.
What changes were proposed in this pull request?
Updated ContainerBalancer#start() and ContainerBalancer#startBalancer() code to assign startedAt after validation runs.
In the existing ContainerBalancer code:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16387
How was this patch tested?
Successful CI : https://github.com/navinko/ozone/actions/runs/35459414407
Unit testing with new test - TestContainerBalancer#testRejectedStartDoesNotModifyStartedAt which verifies that a start request rejected during validation does not update startedAt of last run.
Tested ContainerBalancer locally.
Before Fix
After Fix
Started at from last run did not get overridden.