Skip to content

HDDS-5927. Improve defaults in ContainerBalancerConfiguration #2892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lokeshj1703 merged 10 commits into from
Jan 12, 2022
Merged

HDDS-5927. Improve defaults in ContainerBalancerConfiguration #2892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6170d80
HDDS-5927. Improve defaults in ContainerBalancerConfiguration
siddhantsangwan Dec 7, 2021
1d5072c
Merge branch 'master' into HDDS-5927
siddhantsangwan Dec 13, 2021
36ddd2e
refactor configurations while maintaining backwards compatibility
siddhantsangwan Dec 22, 2021
564d98c
Merge branch 'master' into HDDS-5927
siddhantsangwan Dec 22, 2021
608b5e5
represent threshold as a percentage instead a ratio
siddhantsangwan Dec 22, 2021
63117c5
get ContainerBalancerConfiguration from OzoneConfiguration in the Con…
siddhantsangwan Dec 23, 2021
2a46d52
minor fixes
siddhantsangwan Dec 24, 2021
4ad8901
Merge branch 'master' into HDDS-5927
siddhantsangwan Jan 10, 2022
a692564
handle idleIterations and correct threshold range
siddhantsangwan Jan 10, 2022
3910339
Merge branch 'master' into HDDS-5927
siddhantsangwan Jan 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ Map<String, Pair<Boolean, String>> getSafeModeRuleStatuses()
* Start ContainerBalancer.
*/
boolean startContainerBalancer(Optional<Double> threshold,
Optional<Integer> idleiterations,
Optional<Double> maxDatanodesRatioToInvolvePerIteration,
Optional<Integer> iterations,
Optional<Integer> maxDatanodesPercentageToInvolvePerIteration,
Optional<Long> maxSizeToMovePerIterationInGB,
Optional<Long> maxSizeEnteringTargetInGB,
Optional<Long> maxSizeLeavingSourceInGB) throws IOException;
Expand Down
4 changes: 2 additions & 2 deletions ...n/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ Map<String, Pair<Boolean, String>> getSafeModeRuleStatuses()
* Start ContainerBalancer.
*/
boolean startContainerBalancer(Optional<Double> threshold,
Optional<Integer> idleiterations,
Optional<Double> maxDatanodesRatioToInvolvePerIteration,
Optional<Integer> iterations,
Optional<Integer> maxDatanodesPercentageToInvolvePerIteration,
Optional<Long> maxSizeToMovePerIterationInGB,
Optional<Long> maxSizeEnteringTargetInGB,
Optional<Long> maxSizeLeavingSourceInGB) throws IOException;
Expand Down
34 changes: 17 additions & 17 deletions ...he/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,8 @@ public boolean getReplicationManagerStatus() throws IOException {

@Override
public boolean startContainerBalancer(
Optional<Double> threshold, Optional<Integer> idleiterations,
Optional<Double> maxDatanodesRatioToInvolvePerIteration,
Optional<Double> threshold, Optional<Integer> iterations,
Optional<Integer> maxDatanodesPercentageToInvolvePerIteration,
Optional<Long> maxSizeToMovePerIterationInGB,
Optional<Long> maxSizeEnteringTargetInGB,
Optional<Long> maxSizeLeavingSourceInGB) throws IOException{
Expand All @@ -751,8 +751,8 @@ public boolean startContainerBalancer(
//make balancer configuration optional
if (threshold.isPresent()) {
double tsd = threshold.get();
Preconditions.checkState(tsd >= 0.0D && tsd < 1.0D,
"threshold should to be specified in range [0.0, 1.0).");
Preconditions.checkState(tsd >= 0.0D && tsd < 100D,
"threshold should be specified in range [0.0, 100.0).");
builder.setThreshold(tsd);
}
if (maxSizeToMovePerIterationInGB.isPresent()) {
Expand All @@ -761,22 +761,22 @@ public boolean startContainerBalancer(
"maxSizeToMovePerIterationInGB must be positive.");
builder.setMaxSizeToMovePerIterationInGB(mstm);
}
if (maxDatanodesRatioToInvolvePerIteration.isPresent()) {
double mdti = maxDatanodesRatioToInvolvePerIteration.get();
if (maxDatanodesPercentageToInvolvePerIteration.isPresent()) {
int mdti = maxDatanodesPercentageToInvolvePerIteration.get();
Preconditions.checkState(mdti >= 0,
"maxDatanodesRatioToInvolvePerIteration must be " +
"maxDatanodesPercentageToInvolvePerIteration must be " +
"greater than equal to zero.");
Preconditions.checkState(mdti <= 1,
"maxDatanodesRatioToInvolvePerIteration must be " +
"lesser than equal to one.");
builder.setMaxDatanodesRatioToInvolvePerIteration(mdti);
Preconditions.checkState(mdti <= 100,
"maxDatanodesPercentageToInvolvePerIteration must be " +
"lesser than equal to hundred.");
builder.setMaxDatanodesPercentageToInvolvePerIteration(mdti);
}
if (idleiterations.isPresent()) {
int idi = idleiterations.get();
Preconditions.checkState(idi > 0 || idi == -1,
"idleiterations must be positive or" +
" -1(infinitly run container balancer).");
builder.setIdleiterations(idi);
if (iterations.isPresent()) {
int i = iterations.get();
Preconditions.checkState(i > 0 || i == -1,
"number of iterations must be positive or" +
" -1 (for running container balancer infinitely).");
builder.setIterations(i);
}

if (maxSizeEnteringTargetInGB.isPresent()) {
Expand Down
7 changes: 5 additions & 2 deletions hadoop-hdds/interface-admin/src/main/proto/ScmAdminProtocol.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,14 @@ message GetContainerTokenResponseProto {
message StartContainerBalancerRequestProto {
optional string traceID = 1;
optional double threshold = 2;
optional int32 idleiterations = 3;
optional double maxDatanodesRatioToInvolvePerIteration = 4;
optional int32 idleiterations = 3 [deprecated = true];
optional double maxDatanodesRatioToInvolvePerIteration = 4 [deprecated =
Comment on lines +492 to +493
Copy link
Contributor

@JacksonYao287 JacksonYao287 Dec 23, 2021

Choose a reason for hiding this comment

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

can we just replace them with the new. Compatibility may be not considered for now

Copy link
Contributor Author

@siddhantsangwan siddhantsangwan Dec 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The backwards compatibility goal fails during build if I just replace them.

true];
optional int64 maxSizeToMovePerIterationInGB = 5;
optional int64 maxSizeEnteringTargetInGB = 6;
optional int64 maxSizeLeavingSourceInGB = 7;
optional int32 maxDatanodesPercentageToInvolvePerIteration = 8;
optional int32 iterations = 9;
}

message StartContainerBalancerResponseProto {
Expand Down
39 changes: 20 additions & 19 deletions ...er-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public class ContainerBalancer {
private long maxSizeToMovePerIteration;
private int countDatanodesInvolvedPerIteration;
private long sizeMovedPerIteration;
private int idleIteration;
private int iterations;
private List<DatanodeUsageInfo> unBalancedNodes;
private List<DatanodeUsageInfo> overUtilizedNodes;
private List<DatanodeUsageInfo> underUtilizedNodes;
Expand Down Expand Up @@ -159,6 +159,7 @@ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
if (!validateConfiguration(config)) {
return false;
}
ozoneConfiguration.setFromObject(balancerConfiguration);
balancerRunning = true;
LOG.info("Starting Container Balancer...{}", this);

Expand All @@ -178,23 +179,12 @@ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
* Balances the cluster.
*/
private void balance() {
this.idleIteration = config.getIdleIteration();
if(this.idleIteration == -1) {
this.iterations = config.getIterations();
if(this.iterations == -1) {
//run balancer infinitely
this.idleIteration = Integer.MAX_VALUE;
this.iterations = Integer.MAX_VALUE;
}
this.threshold = config.getThreshold();
this.maxDatanodesRatioToInvolvePerIteration =
config.getMaxDatanodesRatioToInvolvePerIteration();
this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
if (config.getNetworkTopologyEnable()) {
findTargetStrategy = new FindTargetGreedyByNetworkTopology(
containerManager, placementPolicy, nodeManager, networkTopology);
} else {
findTargetStrategy = new FindTargetGreedyByUsageInfo(containerManager,
placementPolicy, nodeManager);
}
for (int i = 0; i < idleIteration && balancerRunning; i++) {
for (int i = 0; i < iterations && balancerRunning; i++) {
// stop balancing if iteration is not initialized
if (!initializeIteration()) {
stop();
Expand All @@ -217,7 +207,7 @@ private void balance() {

// wait for configured time before starting next iteration, unless
// this was the final iteration
if (i != idleIteration - 1) {
if (i != iterations - 1) {
synchronized (this) {
try {
wait(config.getBalancingInterval().toMillis());
Expand Down Expand Up @@ -259,6 +249,17 @@ private boolean initializeIteration() {
}
return false;
}
this.threshold = config.getThresholdAsRatio();
this.maxDatanodesRatioToInvolvePerIteration =
config.getMaxDatanodesRatioToInvolvePerIteration();
this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
if (config.getNetworkTopologyEnable()) {
findTargetStrategy = new FindTargetGreedyByNetworkTopology(
containerManager, placementPolicy, nodeManager, networkTopology);
} else {
findTargetStrategy = new FindTargetGreedyByUsageInfo(containerManager,
placementPolicy, nodeManager);
}
this.excludeNodes = config.getExcludeNodes();
this.includeNodes = config.getIncludeNodes();
// include/exclude nodes from balancing according to configs
Expand Down Expand Up @@ -522,7 +523,7 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
}

/**
* Checks if limits maxDatanodesRatioToInvolvePerIteration and
* Checks if limits maxDatanodesPercentageToInvolvePerIteration and
* maxSizeToMovePerIteration have not been hit.
*
* @return {@link IterationResult#MAX_DATANODES_TO_INVOLVE_REACHED} if reached
Expand Down Expand Up @@ -872,7 +873,7 @@ int getCountDatanodesInvolvedPerIteration() {
return countDatanodesInvolvedPerIteration;
}

long getSizeMovedPerIteration() {
public long getSizeMovedPerIteration() {
return sizeMovedPerIteration;
}

Expand Down
Loading