Skip to content

HDDS-6070. ContainerBalancerConfig doesn't read config from ozone-site.xml #2893

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
ferhui merged 10 commits into from
Dec 28, 2021
Merged

HDDS-6070. ContainerBalancerConfig doesn't read config from ozone-site.xml #2893

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
036f07a
HDDS-6070. ContainerBalancerConfig doesn't read config from ozone-sit…
symious Dec 7, 2021
0a88420
trigger new CI check
symious Dec 8, 2021
4552162
HDDS-6070. Incorrect maxSizeEnteringTarget and maxSizeLeavingSource
symious Dec 10, 2021
f74f473
HDDS-6070. Test the value after initilize
symious Dec 10, 2021
2b0a78c
HDDS-6070. Fix move.timeout
symious Dec 10, 2021
17a1a1c
HDDS-6070. Remove unused imports
symious Dec 10, 2021
19a0b94
HDDS-6070. Remove OzoneConfiguration from ContainerBalancerConfiguration
symious Dec 20, 2021
009dc80
HDDS-6070. Fix some comparision
symious Dec 20, 2021
cb662c8
Merge branch 'master' into HDDS-6070
symious Dec 25, 2021
d0d0615
HDDS-6070. Remove DELTA
symious Dec 25, 2021
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
31 changes: 29 additions & 2 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 @@ -20,6 +20,7 @@

import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.conf.StorageUnit;
import org.apache.hadoop.hdds.fs.DUFactory;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.scm.PlacementPolicy;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
Expand Down Expand Up @@ -124,7 +125,8 @@ public ContainerBalancer(
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
this.config = ozoneConfiguration.
getObject(ContainerBalancerConfiguration.class);
this.metrics = ContainerBalancerMetrics.create();
this.scmContext = scmContext;

Expand Down Expand Up @@ -155,7 +157,7 @@ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {

balancerRunning = true;
this.config = balancerConfiguration;
this.ozoneConfiguration = config.getOzoneConfiguration();
validateConfiguration(config);
LOG.info("Starting Container Balancer...{}", this);

//we should start a new balancer thread async
Expand Down Expand Up @@ -764,6 +766,31 @@ public void stop() {
LOG.info("Container Balancer stopped successfully.");
}

private void validateConfiguration(ContainerBalancerConfiguration conf) {
// maxSizeEnteringTarget and maxSizeLeavingSource should by default be
// greater than container size
long size = (long) ozoneConfiguration.getStorageSize(
ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.BYTES);

if (conf.getMaxSizeEnteringTarget() <= size) {
LOG.info("MaxSizeEnteringTarget should be larger than " +
"ozone.scm.container.size");
}
if (conf.getMaxSizeLeavingSource() <= size) {
LOG.info("MaxSizeLeavingSource should be larger than " +
"ozone.scm.container.size");
}

// balancing interval should be greater than DUFactory refresh period
DUFactory.Conf duConf = ozoneConfiguration.getObject(DUFactory.Conf.class);
long balancingInterval = duConf.getRefreshPeriod().toMillis();
if (conf.getBalancingInterval().toMillis() <= balancingInterval) {
LOG.info("balancing.iteration.interval should be larger than " +
"hdds.datanode.du.refresh.period.");
}
Comment on lines +776 to +791
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.

when validating , if we find it is illegal(for example, conf.getMaxSizeEnteringTarget() <= size), should we throw an Exception and just return without starting balancer?

Copy link
Contributor Author

@symious symious Dec 25, 2021

Choose a reason for hiding this comment

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

Yes, it was intended to return "false" for these situations, but it will break many test cases.
The changes will be brought up in a new ticket.

}

public void setNodeManager(NodeManager nodeManager) {
this.nodeManager = nodeManager;
}
Expand Down
66 changes: 9 additions & 57 deletions ...in/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,10 @@

package org.apache.hadoop.hdds.scm.container.balancer;

import com.google.common.base.Preconditions;
import org.apache.hadoop.hdds.conf.Config;
import org.apache.hadoop.hdds.conf.ConfigGroup;
import org.apache.hadoop.hdds.conf.ConfigTag;
import org.apache.hadoop.hdds.conf.ConfigType;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.conf.StorageUnit;
import org.apache.hadoop.hdds.fs.DUFactory;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.hdds.scm.container.ContainerID;
import org.apache.hadoop.ozone.OzoneConsts;
import org.slf4j.Logger;
Expand All @@ -37,7 +32,6 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

/**
Expand All @@ -47,7 +41,6 @@
public final class ContainerBalancerConfiguration {
private static final Logger LOG =
LoggerFactory.getLogger(ContainerBalancerConfiguration.class);
private OzoneConfiguration ozoneConfiguration;

@Config(key = "utilization.threshold", type = ConfigType.AUTO, defaultValue =
"0.1", tags = {ConfigTag.BALANCER},
Expand All @@ -73,19 +66,19 @@ public final class ContainerBalancerConfiguration {
private long maxSizeToMovePerIteration = 30 * OzoneConsts.GB;

@Config(key = "size.entering.target.max", type = ConfigType.SIZE,
defaultValue = "", tags = {ConfigTag.BALANCER}, description = "The " +
defaultValue = "26GB", tags = {ConfigTag.BALANCER}, description = "The " +
"maximum size that can enter a target datanode in each " +
"iteration while balancing. This is the sum of data from multiple " +
"sources. The default value is greater than the configured" +
" (or default) ozone.scm.container.size by 1GB.")
"sources. The value must be greater than the configured" +
" (or default) ozone.scm.container.size.")
private long maxSizeEnteringTarget;

@Config(key = "size.leaving.source.max", type = ConfigType.SIZE,
defaultValue = "", tags = {ConfigTag.BALANCER}, description = "The " +
defaultValue = "26GB", tags = {ConfigTag.BALANCER}, description = "The " +
"maximum size that can leave a source datanode in each " +
"iteration while balancing. This is the sum of data moving to multiple " +
"targets. The default value is greater than the configured" +
" (or default) ozone.scm.container.size by 1GB.")
"targets. The value must be greater than the configured" +
" (or default) ozone.scm.container.size.")
private long maxSizeLeavingSource;

@Config(key = "idle.iterations", type = ConfigType.INT,
Expand All @@ -99,13 +92,13 @@ public final class ContainerBalancerConfiguration {
private String excludeContainers = "";

@Config(key = "move.timeout", type = ConfigType.TIME, defaultValue = "30m",
timeUnit = TimeUnit.MINUTES, tags = {ConfigTag.BALANCER}, description =
tags = {ConfigTag.BALANCER}, description =
"The amount of time in minutes to allow a single container to move " +
"from source to target.")
private long moveTimeout = Duration.ofMinutes(30).toMillis();

@Config(key = "balancing.iteration.interval", type = ConfigType.TIME,
defaultValue = "1h", timeUnit = TimeUnit.MINUTES, tags = {
defaultValue = "70m", tags = {
ConfigTag.BALANCER}, description = "The interval period between each " +
"iteration of Container Balancer.")
private long balancingInterval;
Expand All @@ -131,33 +124,6 @@ public final class ContainerBalancerConfiguration {
"This configuration is false by default.")
private boolean networkTopologyEnable = false;

private DUFactory.Conf duConf;

/**
* Create configuration with default values.
*
* @param config Ozone configuration
*/
public ContainerBalancerConfiguration(OzoneConfiguration config) {
Preconditions.checkNotNull(config,
"OzoneConfiguration should not be null.");
this.ozoneConfiguration = config;

// maxSizeEnteringTarget and maxSizeLeavingSource should by default be
// greater than container size
long size = (long) ozoneConfiguration.getStorageSize(
ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT, StorageUnit.GB) +
OzoneConsts.GB;
maxSizeEnteringTarget = size;
maxSizeLeavingSource = size;

// balancing interval should be greater than DUFactory refresh period
duConf = ozoneConfiguration.getObject(DUFactory.Conf.class);
balancingInterval = duConf.getRefreshPeriod().toMillis() +
Duration.ofMinutes(10).toMillis();
}

/**
* Gets the threshold value for Container Balancer.
*
Expand Down Expand Up @@ -324,12 +290,7 @@ public Duration getBalancingInterval() {
}

public void setBalancingInterval(Duration balancingInterval) {
if (balancingInterval.toMillis() > duConf.getRefreshPeriod().toMillis()) {
this.balancingInterval = balancingInterval.toMillis();
} else {
LOG.warn("Balancing interval duration must be greater than du refresh " +
"period, {} milliseconds", duConf.getRefreshPeriod().toMillis());
}
this.balancingInterval = balancingInterval.toMillis();
}

/**
Expand Down Expand Up @@ -381,15 +342,6 @@ public void setExcludeNodes(String excludeNodes) {
this.excludeNodes = excludeNodes;
}

/**
* Gets the {@link OzoneConfiguration} using which this configuration was
* constructed.
* @return the {@link OzoneConfiguration} being used by this configuration
*/
public OzoneConfiguration getOzoneConfiguration() {
return this.ozoneConfiguration;
}

@Override
public String toString() {
return String.format("Container Balancer Configuration values:%n" +
Expand Down
2 changes: 1 addition & 1 deletion ...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ public boolean startContainerBalancer(
Optional<Long> maxSizeLeavingSource) throws IOException {
getScm().checkAdminAccess(getRemoteUser());
ContainerBalancerConfiguration cbc =
new ContainerBalancerConfiguration(scm.getConfiguration());
scm.getConfiguration().getObject(ContainerBalancerConfiguration.class);
if (threshold.isPresent()) {
double tsd = threshold.get();
Preconditions.checkState(tsd >= 0.0D && tsd < 1.0D,
Expand Down
26 changes: 23 additions & 3 deletions ...cm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ public void setup() throws SCMException, NodeNotFoundException {
containerManager = Mockito.mock(ContainerManager.class);
replicationManager = Mockito.mock(ReplicationManager.class);

balancerConfiguration = new ContainerBalancerConfiguration(conf);
balancerConfiguration =
conf.getObject(ContainerBalancerConfiguration.class);
balancerConfiguration.setThreshold(0.1);
balancerConfiguration.setIdleIteration(1);
balancerConfiguration.setMaxDatanodesRatioToInvolvePerIteration(1.0d);
Expand Down Expand Up @@ -488,8 +489,10 @@ public void balancerShouldObeyMaxSizeEnteringTargetLimit() {
Assert.assertTrue(containerBalancer.getSourceToTargetMap().isEmpty());

// some containers should be selected when using default values
containerBalancer.start(
new ContainerBalancerConfiguration(new OzoneConfiguration()));
OzoneConfiguration ozoneConfiguration = new OzoneConfiguration();
ContainerBalancerConfiguration cbc = ozoneConfiguration.
getObject(ContainerBalancerConfiguration.class);
containerBalancer.start(cbc);

// waiting for balance completed.
// TODO: this is a temporary implementation for now
Expand Down Expand Up @@ -591,6 +594,23 @@ public void balancerShouldFollowExcludeAndIncludeDatanodesConfigurations() {
}
}

@Test
public void testContainerBalancerConfiguration() {
OzoneConfiguration ozoneConfiguration = new OzoneConfiguration();
ozoneConfiguration.set("ozone.scm.container.size", "5GB");
ozoneConfiguration.setDouble(
"hdds.container.balancer.utilization.threshold", 0.01);

ContainerBalancerConfiguration cbConf =
ozoneConfiguration.getObject(ContainerBalancerConfiguration.class);
Assert.assertEquals(cbConf.getThreshold(), 0.01d, 0.001);

Assert.assertEquals(cbConf.getMaxSizeLeavingSource(),
26 * 1024 * 1024 * 1024L);

Assert.assertEquals(cbConf.getMoveTimeout().toMillis(), 30 * 60 * 1000);
}

/**
* Determines unBalanced nodes, that is, over and under utilized nodes,
* according to the generated utilization values for nodes and the threshold.
Expand Down