Skip to content

HDDS-15278. DiskBalancer should validate persisted config before applying.#10271

Merged
ChenSammi merged 2 commits into
apache:masterfrom
slfan1989:HDDS-15278
May 15, 2026
Merged

HDDS-15278. DiskBalancer should validate persisted config before applying.#10271
ChenSammi merged 2 commits into
apache:masterfrom
slfan1989:HDDS-15278

Conversation

@slfan1989
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR validates persisted DiskBalancer configuration before applying it to the running service.

DiskBalancerService loads persisted configuration from diskbalancer.info and applies it through applyDiskBalancerInfo. Previously, only containerStates was explicitly validated in this path, while other fields such as threshold, bandwidthInMB, and parallelThread were applied directly from DiskBalancerInfo.

This PR updates applyDiskBalancerInfo to convert DiskBalancerInfo through DiskBalancerConfiguration, so the existing validation logic is reused consistently before updating the service state.

It also adds unit test coverage to verify that invalid persisted DiskBalancer configuration is rejected, including:

  • invalid threshold
  • invalid bandwidth
  • invalid parallel thread count

What is the link to the Apache JIRA

JIRA: HDDS-15278. DiskBalancer should validate persisted config before applying.

How was this patch tested?

Add Some Junit Test.

@slfan1989
Copy link
Copy Markdown
Contributor Author

@Gargi-jais11 @ChenSammi Could you please help review this PR when you have time? This patch makes DiskBalancer validate persisted diskbalancer.info configuration before applying it, and adds tests for invalid persisted values. Thanks!

Copy link
Copy Markdown
Contributor

@Gargi-jais11 Gargi-jais11 left a comment

Choose a reason for hiding this comment

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

Thanks @slfan1989 for catching this. Please find the inlined comment.

Copy link
Copy Markdown
Contributor

@Gargi-jais11 Gargi-jais11 left a comment

Choose a reason for hiding this comment

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

+1, LGTM.

Copy link
Copy Markdown
Contributor

@ChenSammi ChenSammi left a comment

Choose a reason for hiding this comment

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

Thanks @slfan1989 , LGTM.

@ChenSammi ChenSammi merged commit 0db8704 into apache:master May 15, 2026
47 checks passed
@slfan1989
Copy link
Copy Markdown
Contributor Author

@Gargi-jais11 @ChenSammi Thanks for the review and for merging the PR!

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