HDDS-6280. Support Container Balancer HA #3423

siddhantsangwan · 2022-05-16T14:35:39Z

What changes were proposed in this pull request?

Make Container Balancer highly available by persisting its configurations (state) through RocksDB and replicating through Ratis. This change introduces a new protobuf message, ContainerBalancerConfiguration, containing configurations that need to be persisted. On leader change or restart, ContainerBalancer checks if it needs to start by reading this persisted ContainerBalancerConfiguration proto in the new leader.

What is the link to the Apache JIRA

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

How was this patch tested?

Introduced a new test TestFailoverWithSCMHA#testContainerBalancerPersistsConfigurationInAllSCMs()

…tartBalancer. This ensures that balancer starts with the 0th iteration when ContainerBalancer#startBalancer is called.

lokeshj1703

@siddhantsangwan Thanks for working on this! I have a few comments inline.

hadoop-hdds/interface-client/src/main/proto/hdds.proto

...erver-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java

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

siddhantsangwan · 2022-05-20T10:15:39Z

I think we need to somehow clearly distinguish between the ContainerBalancer#startBalancer and SCMService#start methods. startBalancer is a command for persisting configuration and starting balancer. start should be used when there's been a leader change or restart - to read configuration and start balancer.

…r in test

JacksonYao287

thanks @siddhantsangwan for this work! i have a few comments inline, PTAL!

JacksonYao287 · 2022-05-23T10:35:26Z

...erver-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java

+        // balancer has been stopped in another thread
+        if (!isBalancerRunning()) {
+          return;
+        }


can we add a balancer running check currentThread = null inside stopBalancer, so that the two operations will be protected by a single lock. if we first call isBalancerRunning() and then call stopBalancer() , there might be a case that between the two operation in a single thread, stopbalancer is called from another thread.

stopBalancer calls validateState(true), which checks if balancer is currently running. Is this what you're looking for?

JacksonYao287 · 2022-05-23T10:41:03Z

...erver-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java

  @Override
  public boolean shouldRun() {
-    return false;
+    try {


should we check the status and the delay here?

public boolean shouldRun() { serviceLock.lock(); try { // If safe mode is off, then this SCMService starts to run with a delay. return serviceStatus == ServiceStatus.RUNNING && clock.millis() - lastTimeToBeReadyInMillis >= waitTimeInMillis; } finally { serviceLock.unlock(); } }

Since we're already reading the persisted status for telling whether balancer should run, we probably don't need to check and update ServiceStatus. The persisted status is the most reliable information in balancer's case since it's not a background service.

JacksonYao287 · 2022-05-23T10:45:17Z

...erver-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java

+    lock.lock();
+    try {
+      if (!scmContext.isLeader() || scmContext.isInSafeMode()) {
+        if (isBalancerRunning()) {


should we check and set the ServiceStatus here? for example

if (scmContext.isLeaderReady() && !scmContext.isInSafeMode()) { if (serviceStatus != ServiceStatus.RUNNING) { LOG.info("Service {} transitions to RUNNING.", getServiceName()); serviceStatus = ServiceStatus.RUNNING; lastTimeToBeReadyInMillis = clock.millis(); } } else { if (serviceStatus != ServiceStatus.PAUSING) { LOG.info("Service {} transitions to PAUSING.", getServiceName()); serviceStatus = ServiceStatus.PAUSING; } }

In addition to my previous reply, I think holding state in ServiceStatus along with persisting it in RocksDB would make the logic a bit complex. We'd then have three ways of checking state - ServiceStatus, RocksDB, and checking the current thread for null. What do you think @JacksonYao287 ?

…ncerConfigOnStartBalancer

…BalancerPersistsConfigurationInAllSCMs

siddhantsangwan · 2022-05-25T10:58:41Z

@lokeshj1703 @JacksonYao287 thanks for reviewing! I've updated the PR.

siddhantsangwan · 2022-05-26T05:54:20Z

During manual testing in a cluster, I discovered that if a Storage Container Manager process is stopped, it would lead to balancer also stopping because of containerBalancer.stopBalancer(); being called in StorageContainerManager#stop. This is undesirable since we want balancer to start running in the new leader. I'm now testing if calling containerBalancer.stop() instead has the desired effect.

siddhantsangwan · 2022-05-31T04:42:51Z

@JacksonYao287 @lokeshj1703 @nandakumar131 After the latest changes, balancer seems to be working as expected.

lokeshj1703

@siddhantsangwan Thanks for updating the PR! I have few minor comments.

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ByteStringCodec.java

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

lokeshj1703

@siddhantsangwan Thanks for updating the PR! The changes look good to me. +1.

lokeshj1703 · 2022-06-01T08:29:19Z

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ByteStringCodec.java

+/**
+ * A dummy codec that serializes a ByteString object to ByteString.
+ */
+public class ByteStringCodec implements Codec {


We already have a ByteStringCodec class. Is it possible to reuse it?

siddhantsangwan · 2022-06-01T12:20:50Z

Thanks for the reviews! I've merged this PR to master.

* master: (87 commits) HDDS-6686. Do Leadship check before SASL token verification. (apache#3382) HDDS-4364: [FSO]List FileStatus : startKey can be a non-existed path (apache#3481) HDDS-6091. Add file checksum to OmKeyInfo (apache#3201) HDDS-6706. Exposing Volume Information Metrics to the DataNode UI (apache#3478) HDDS-6759: Add listblock API in MockDatanodeStorage (apache#3452) HDDS-5821 Container cache management for closing RockDB (apache#3426) HDDS-6683. Refactor OM server bucket layout configuration usage (apache#3477) HDDS-6824. Revert changes made in proto.lock by HDDS-6768. (apache#3480) HDDS-6811. Bucket create message with layout type (apache#3479) HDDS-6810. Add a optional flag to trigger listStatus as part of listKeys for FSO buckets. (apache#3461) HDDS-6828. Revert RockDB version pending leak fixes (apache#3475) HDDS-6764: EC: DN ability to create RECOVERING containers for EC reconstruction. (apache#3458) HDDS-6795: EC: PipelineStateMap#addPipeline should not have precondition checks post db updates (apache#3453) HDDS-6823. Intermittent failure in TestOzoneECClient#testExcludeOnDNMixed (apache#3476) HDDS-6820. Bucket Layout Post-Finalization Validators for ACL Requests. (apache#3472) HDDS-6819. Add LEGACY to AllowedBucketLayouts in CreateBucketHandler (apache#3473) HDDS-4859. [FSO]ListKeys: seek all the files/dirs from startKey to keyPrefix (apache#3466) HDDS-6705 Add metrics for volume statistics including disk capacity, usage, Reserved (apache#3430) HDDS-6474. Add test to cover the FSO bucket list status with beyond batch boundary and cache. (apache#3379). Contributed by aswinshakil HDDS-6280. Support Container Balancer HA (apache#3423) ...

HDDS-6280. Support Container Balancer HA

3933e57

siddhantsangwan requested review from JacksonYao287, lokeshj1703 and nandakumar131 May 16, 2022 14:36

siddhantsangwan added 3 commits May 17, 2022 11:27

Set nextIterationIndex to 0 in ContainerBalancer#setBalancerConfigOnS…

c7633ba

…tartBalancer. This ensures that balancer starts with the 0th iteration when ContainerBalancer#startBalancer is called.

improve exception handling

b1a80d7

minor change

4b7cd67

lokeshj1703 reviewed May 19, 2022

View reviewed changes

siddhantsangwan added 7 commits May 20, 2022 16:41

register ContainerBalancer with SCMServiceManager

bc99004

test that configuration is persisted in all SCMs

0501ef6

mock SCMServiceManager in TestContainerBalancer

276d748

Merge branch 'TestContainerBalancerHA' into HDDS-6280

93dbdff

rat and checkstyle

6cbdf21

trigger new CI check

c65f297

address review comments and wait for followers to catch up with leade…

d0c56b3

…r in test

JacksonYao287 reviewed May 23, 2022

View reviewed changes

siddhantsangwan added 5 commits May 23, 2022 17:47

save reference to balancer configuration in ContainerBalancer#setBala…

f2c279e

…ncerConfigOnStartBalancer

Merge branch 'master' into HDDS-6280

d1d090f

update testContainerBalancerPersistsConfigurationInAllSCMs

f592ebd

add timed checks for isBalancerRunning and shouldRun in testContainer…

61620cc

…BalancerPersistsConfigurationInAllSCMs

introduced ContainerBalancer#tryStopBalancer for better code reuse

4b40d99

siddhantsangwan added 2 commits May 26, 2022 14:19

use stop instead of stopBalancer in StorageContainerManager#stop

066f284

Merge branch 'master' into HDDS-6280

20fe445

lokeshj1703 reviewed Jun 1, 2022

View reviewed changes

add checks for LOG.isDebugEnabled()

58f4e81

lokeshj1703 approved these changes Jun 1, 2022

View reviewed changes

siddhantsangwan merged commit e8abd0f into apache:master Jun 1, 2022

HDDS-6280. Support Container Balancer HA #3423

HDDS-6280. Support Container Balancer HA #3423

Uh oh!

Conversation

siddhantsangwan commented May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

lokeshj1703 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!

siddhantsangwan commented May 20, 2022

Uh oh!

JacksonYao287 left a comment

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 May 23, 2022

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan May 24, 2022

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 May 23, 2022

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan May 25, 2022

Choose a reason for hiding this comment

Uh oh!

JacksonYao287 May 23, 2022

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan May 25, 2022

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan commented May 25, 2022

Uh oh!

siddhantsangwan commented May 26, 2022

Uh oh!

siddhantsangwan commented May 31, 2022

Uh oh!

lokeshj1703 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lokeshj1703 left a comment

Choose a reason for hiding this comment

Uh oh!

lokeshj1703 Jun 1, 2022

Choose a reason for hiding this comment

Uh oh!

siddhantsangwan commented Jun 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

siddhantsangwan commented May 16, 2022 •

edited

Loading