Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,14 @@ public ContainerBalancerTask(StorageContainerManager scm,
this.selectedSources = new HashSet<>();
this.selectedTargets = new HashSet<>();
findSourceStrategy = new FindSourceGreedy(nodeManager);
if (config.getNetworkTopologyEnable()) {
findTargetStrategy = new FindTargetGreedyByNetworkTopology(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to make field findTargetStrategy final

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @Montura. In the interest of time, let's handle this in the next patch.

containerManager, placementPolicyValidateProxy,
nodeManager, networkTopology);
} else {
findTargetStrategy = new FindTargetGreedyByUsageInfo(containerManager,
placementPolicyValidateProxy, nodeManager);
}
this.iterationsStatistic = new ArrayList<>();
}

Expand Down Expand Up @@ -432,14 +440,7 @@ private boolean initializeIteration() {
this.maxDatanodesRatioToInvolvePerIteration =
config.getMaxDatanodesRatioToInvolvePerIteration();
this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
if (config.getNetworkTopologyEnable()) {
findTargetStrategy = new FindTargetGreedyByNetworkTopology(
containerManager, placementPolicyValidateProxy,
nodeManager, networkTopology);
} else {
findTargetStrategy = new FindTargetGreedyByUsageInfo(containerManager,
placementPolicyValidateProxy, nodeManager);
}

this.excludeNodes = config.getExcludeNodes();
this.includeNodes = config.getIncludeNodes();
// include/exclude nodes from balancing according to configs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public MockedSCM(@Nonnull TestableCluster testableCluster) {
}
}

private void init(@Nonnull ContainerBalancerConfiguration balancerConfig, @Nonnull OzoneConfiguration ozoneCfg) {
void init(@Nonnull ContainerBalancerConfiguration balancerConfig, @Nonnull OzoneConfiguration ozoneCfg) {
ozoneCfg.setFromObject(balancerConfig);
try {
doMock(balancerConfig, ozoneCfg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
package org.apache.hadoop.hdds.scm.container.balancer;

import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
import org.apache.hadoop.ozone.OzoneConsts;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.util.List;
Expand Down Expand Up @@ -55,4 +57,30 @@ void testGetIterationStatistics() {
});

}

/**
* @see <a href="https://issues.apache.org/jira/browse/HDDS-11350">HDDS-11350</a>
*/
@Test
void testGetCurrentIterationsStatisticDoesNotThrowNullPointerExceptionWhenBalancingThreadIsSleeping() {
MockedSCM mockedScm = new MockedSCM(new TestableCluster(10, OzoneConsts.GB));
OzoneConfiguration ozoneConfig = new OzoneConfiguration();
ContainerBalancerConfiguration config = ozoneConfig.getObject(ContainerBalancerConfiguration.class);

config.setIterations(2);
// the following config makes the balancing thread go to sleep while waiting for DU to be triggered in DNs and
// updated storage reports to arrive via DN heartbeats - of course, this is a unit test and NodeManager, DNs etc.
// are all mocked
config.setTriggerDuEnable(true);
mockedScm.init(config, ozoneConfig);

// run ContainerBalancerTask in a new thread and have the current thread call getCurrentIterationsStatistic
StorageContainerManager scm = mockedScm.getStorageContainerManager();
ContainerBalancer cb = new ContainerBalancer(scm);
ContainerBalancerTask task = new ContainerBalancerTask(scm, 0, cb, cb.getMetrics(), config, false);
Thread thread = new Thread(task);
thread.setDaemon(true);
thread.start();
Assertions.assertDoesNotThrow(task::getCurrentIterationsStatistic);
}
}