rakeshadr commented on code in PR #11000:
URL: https://github.com/apache/ozone/pull/11000#discussion_r3793592406
##########
hadoop-hdds/docs/content/feature/ContainerBalancer.md:
##########
@@ -101,7 +101,7 @@ The Container Balancer can also be configured through the
`ozone-site.xml` file.
| `hdds.container.balancer.exclude.containers` | A comma-separated
list of container IDs to exclude from balancing.
| "" |
| `hdds.container.balancer.move.timeout` | The amount of time
to allow a single container to move from source to target.
| 65m |
| `hdds.container.balancer.move.replication.timeout` | The amount of time
to allow a single container's replication from source to target as part of a
container move. | 50m |
-| `hdds.container.balancer.balancing.iteration.interval` | The interval period
between each iteration of Container Balancer.
| 70m |
+| `hdds.container.balancer.balancing.iteration.interval` | The interval period
between each iteration of Container Balancer.
| 3m |
Review Comment:
Can you improve description with operation details.
| `hdds.container.balancer.balancing.iteration.interval` | The interval to
wait between iterations. The balancer runs continuously, this wait lets SCM
receive refreshed Datanode usage information before the next iteration starts.
| 3m |
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java:
##########
Review Comment:
Can you add additional log info to improve debuggability.
```
LOG.info("Container Balancer starting with balancing iteration interval {}
seconds " +
"(hdds.container.balancer.balancing.iteration.interval).",
configuration.getBalancingInterval(). toSeconds());
```
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java:
##########
@@ -112,9 +112,11 @@ public final class ContainerBalancerConfiguration {
private long moveReplicationTimeout = Duration.ofMinutes(50).toMillis();
@Config(key = "hdds.container.balancer.balancing.iteration.interval", type =
ConfigType.TIME,
- defaultValue = "70m", tags = {ConfigTag.BALANCER}, description =
- "The interval period between each iteration of Container Balancer.")
- private long balancingInterval = Duration.ofMinutes(70).toMillis();
+ defaultValue = "3m", tags = {ConfigTag.BALANCER}, description =
Review Comment:
@ashishkumar50 The current codebase has the metrics for detecting problems
(failure rate, timeout rate) but no mechanism to react to them automatically.
IMHO, the existing balancer infrastructure to be improved before making the
interval shorter.
1) Per-failure-type metrics — fix the TODO in ContainerBalancerMetrics
2) Exponential backoff on high failure rate — in ContainerBalancerTask
inter-iteration sleep
3) Introduce balancer bandwidth similar to HDFS, this was discussed earlier.
Please create separate jira tasks, as a pre-requisite or follow-up tasks as
your wish.
**_More details here:_**
**Point-1)**
For example, we have a TODO item. Can you please explore and improve more
balancer metrics to catch balancing operation health.
https://github.com/ashishkumar50/ozone/blob/a2734b22750e0241dab2979cbd2062dec173d993/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java#L205
**Point-2)**
Instead of always sleeping for the configured interval, how about check the
possibility of adding a backoff check ? Adding a sample reference logic as a
brainstorming idea (vibe coded, fyi). Please think in this line so that users
can continuously run it without much worries.
```
// After each iteration, before sleeping, evaluate health
int consecutiveHighFailureIterations = 0;
final double FAILURE_RATE_THRESHOLD = 0.5; // >50% moves failing = back off
// ... inside the iteration loop, after doIteration() ...
long scheduled = metrics.getNumContainerMovesScheduledInLatestIteration();
long failed = metrics.getNumContainerMovesFailedInLatestIteration();
long timedOut = metrics.getNumContainerMovesTimeoutInLatestIteration();
double failureRate = (scheduled > 0) ? (double)(failed + timedOut) /
scheduled : 0;
if (failureRate > FAILURE_RATE_THRESHOLD) {
consecutiveHighFailureIterations++;
long backoffMs = config.getBalancingInterval().toMillis()
* (1L << Math.min(consecutiveHighFailureIterations, 4)); // cap at 16×
LOG.warn("Container Balancer failure rate {:.1f}% exceeds threshold. " +
"Backing off for {}ms before next iteration.",
failureRate * 100, backoffMs);
Thread.sleep(backoffMs);
} else {
consecutiveHighFailureIterations = 0;
Thread.sleep(config.getBalancingInterval().toMillis());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
