HDDS-10358. Allow Container Balancer tuning options to be set in CLI #6241

sarvekshayr · 2024-02-21T03:50:27Z

What changes were proposed in this pull request?

Introducing more configuration options in the Container Balancer CLI tool. Options for including and excluding nodes from balancing, "balancing.iteration.interval", "move.timeout", "move.replication.timeout" for managing timeouts and "move.networkTopology.enable" for enabling network topology aware moves.

What is the link to the Apache JIRA

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

How was this patch tested?

Listing all the subcommands under containerbalancer start

Default values of the tuning options

Starting containerbalancer with the newly added tuning options in CLI.

errose28

Thanks for working on this @sarvekshayr. Let's also add a test that when CLI options and configurations are present, the CLI options override the defined configurations (assuming that is expected behavior).

.../main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java

siddhantsangwan

Thanks for working on this. I've left some comments.

...pache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java

.../main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java

siddhantsangwan · 2024-02-27T04:18:42Z

@SaketaChalamchala would you like to review?

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

sumitagrawl

@sarvekshayr Thanks for working over this, have few minor comments.

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java

.../integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java

Tejaskriya

Overall the changes look good! Just a few improvements, you can find the comments below

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java

tanvipenumudy

Thank you @sarvekshayr for updating the patch, please find a few minor nits.

.../integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java

…ozone/TestContainerBalancerOperations.java Co-authored-by: tanvipenumudy <46785609+tanvipenumudy@users.noreply.github.com>

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java

.../integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java

sumitagrawl

LGTM +1

Tejaskriya

Please find a few comments regarding the checks changed.

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

Tejaskriya

LGTM, +1. Thanks for making the changes

siddhantsangwan

@sarvekshayr Thanks for updating. Other than my comment above about error handling, this change looks good now. We can kick off CI once you've updated it.

siddhantsangwan

Waiting for green CI.

siddhantsangwan · 2024-04-01T05:05:27Z

Thanks @sarvekshayr and all the reviewers. Merging to master.

…pache#6241) (cherry picked from commit 077fff4)

HDDS-10358. Allow Container Balancer tuning options to be set in CLI

f53937f

sarvekshayr marked this pull request as draft February 21, 2024 03:50

sarvekshayr added 2 commits February 21, 2024 17:10

Fixed checkstyle errors

2f044e2

Unintentional changes reverted

284fa5f

sarvekshayr marked this pull request as ready for review February 21, 2024 13:38

adoroszlai requested a review from siddhantsangwan February 21, 2024 18:57

errose28 reviewed Feb 21, 2024

View reviewed changes

.../main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java Outdated Show resolved Hide resolved

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java Outdated Show resolved Hide resolved

siddhantsangwan reviewed Feb 22, 2024

View reviewed changes

errose28 reviewed Feb 27, 2024

View reviewed changes

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java Outdated Show resolved Hide resolved

sumitagrawl reviewed Feb 28, 2024

View reviewed changes

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java Outdated Show resolved Hide resolved

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java Outdated Show resolved Hide resolved

sarvekshayr marked this pull request as draft February 28, 2024 10:43

sarvekshayr added 4 commits February 28, 2024 16:20

Addressed the comments - 1

73e414d

Addressed the comments - 2

4ab4e31

Integration test and some changes

0c76fda

Fixed checkstyle errors - 2

ee76f13

tanvipenumudy reviewed Mar 6, 2024

View reviewed changes

.../integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java Show resolved Hide resolved

sarvekshayr added 2 commits March 6, 2024 19:55

Increased Timeout value

23c33e4

Fixed checkstyle errors - 3

22f866a

sarvekshayr marked this pull request as ready for review March 6, 2024 15:45

sarvekshayr requested review from siddhantsangwan, sumitagrawl and tanvipenumudy March 6, 2024 15:45

Tejaskriya reviewed Mar 6, 2024

View reviewed changes

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java Outdated Show resolved Hide resolved

...dds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStartSubcommand.java Outdated Show resolved Hide resolved

sarvekshayr requested a review from errose28 March 7, 2024 03:31

tanvipenumudy reviewed Mar 13, 2024

View reviewed changes

sarvekshayr and others added 4 commits March 13, 2024 11:26

Update hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/…

2973df6

…ozone/TestContainerBalancerOperations.java Co-authored-by: tanvipenumudy <46785609+tanvipenumudy@users.noreply.github.com>

Update hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/…

a83bb93

…ozone/TestContainerBalancerOperations.java Co-authored-by: tanvipenumudy <46785609+tanvipenumudy@users.noreply.github.com>

Update hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/…

5c5bbbc

…ozone/TestContainerBalancerOperations.java Co-authored-by: tanvipenumudy <46785609+tanvipenumudy@users.noreply.github.com>

Added default value for all params

7dc8b44

siddhantsangwan reviewed Mar 14, 2024

View reviewed changes

Changed long to int for time variables

1ac1608

sumitagrawl approved these changes Mar 22, 2024

View reviewed changes

sarvekshayr added 3 commits March 22, 2024 15:50

Server side SCMException

b9260a0

Fixed checkstyle errors - 4

f3fcd70

Added proto and improved Integration test

1d0b3b2

Tejaskriya reviewed Mar 25, 2024

View reviewed changes

Corrected the checks

d041fc3

Tejaskriya approved these changes Mar 26, 2024

View reviewed changes

siddhantsangwan reviewed Mar 27, 2024

View reviewed changes

Changed to IOException

05c771a

siddhantsangwan approved these changes Mar 27, 2024

View reviewed changes

siddhantsangwan merged commit 077fff4 into apache:master Apr 1, 2024

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024

HDDS-10358. Allow Container Balancer tuning options to be set in CLI (a…

90504e0

…pache#6241) (cherry picked from commit 077fff4)

HDDS-10358. Allow Container Balancer tuning options to be set in CLI #6241

HDDS-10358. Allow Container Balancer tuning options to be set in CLI #6241

Uh oh!

Conversation

sarvekshayr commented Feb 21, 2024

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

errose28 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

siddhantsangwan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

siddhantsangwan commented Feb 27, 2024

Uh oh!

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Tejaskriya left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tanvipenumudy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

Tejaskriya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Tejaskriya left a comment

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan left a comment

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan left a comment

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan commented Apr 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Tejaskriya left a comment •

edited

Loading