adoroszlai commented on code in PR #7580:
URL: https://github.com/apache/ozone/pull/7580#discussion_r1885742459
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -1456,46 +1402,8 @@ public Clock getClock() {
return clock;
}
- /**
- * following functions will be refactored in a separate jira.
- */
- public CompletableFuture<MoveManager.MoveResult> move(
- ContainerID cid, DatanodeDetails src, DatanodeDetails tgt)
- throws NodeNotFoundException, ContainerNotFoundException,
- TimeoutException {
- CompletableFuture<MoveManager.MoveResult> ret =
- new CompletableFuture<>();
- if (!isRunning()) {
- ret.complete(MoveManager.MoveResult.FAIL_UNEXPECTED_ERROR);
- LOG.warn("Failing move because Replication Monitor thread's " +
- "running state is {}", isRunning());
- return ret;
- }
-
- return legacyReplicationManager.move(cid, src, tgt);
- }
-
- public Map<ContainerID, CompletableFuture<MoveManager.MoveResult>>
- getInflightMove() {
- return legacyReplicationManager.getInflightMove();
- }
-
- public LegacyReplicationManager.MoveScheduler getMoveScheduler() {
- return legacyReplicationManager.getMoveScheduler();
- }
-
- @VisibleForTesting
- public LegacyReplicationManager getLegacyReplicationManager() {
- return legacyReplicationManager;
- }
-
public boolean isContainerReplicatingOrDeleting(ContainerID containerID) {
- if (rmConf.isLegacyEnabled()) {
- return legacyReplicationManager
- .isContainerReplicatingOrDeleting(containerID);
- } else {
return !getPendingReplicationOps(containerID).isEmpty();
Review Comment:
```
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java
1406: 'method def' child has incorrect indentation level 6, expected level
should be 4.
```
```suggestion
return !getPendingReplicationOps(containerID).isEmpty();
```
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java:
##########
@@ -437,29 +433,19 @@ public void
targetDatanodeShouldBeInServiceHealthy(@Nonnull MockedSCM mockedSCM)
@ParameterizedTest(name = "MockedSCM #{index}: {0}")
@MethodSource("createMockedSCMs")
public void selectedContainerShouldNotAlreadyHaveBeenSelected(@Nonnull
MockedSCM mockedSCM)
- throws NodeNotFoundException, ContainerNotFoundException,
TimeoutException, ContainerReplicaNotFoundException {
+ throws NodeNotFoundException, ContainerNotFoundException,
ContainerReplicaNotFoundException {
ContainerBalancerConfiguration config = new
ContainerBalancerConfigBuilder(mockedSCM.getNodeCount()).build();
- mockedSCM.enableLegacyReplicationManager();
-
- ContainerBalancerTask task = mockedSCM.startBalancerTask(config);
- int numContainers = task.getContainerToTargetMap().size();
-
/* Assuming move is called exactly once for each unique container, number
of calls to move should equal number of
unique containers. If number of calls to move is more than number of
unique containers, at least one container
has been re-selected. It's expected that number of calls to move should
equal number of unique, selected containers
(from containerToTargetMap).
*/
- verify(mockedSCM.getReplicationManager(), times(numContainers))
- .move(any(ContainerID.class), any(DatanodeDetails.class),
any(DatanodeDetails.class));
-
- // Try the same test by disabling LegacyReplicationManager so that
MoveManager is used.
- mockedSCM.disableLegacyReplicationManager();
ContainerBalancerTask nextTask = mockedSCM.startBalancerTask(config);
- numContainers = nextTask.getContainerToTargetMap().size();
+ int numContainers = nextTask.getContainerToTargetMap().size();
verify(mockedSCM.getMoveManager(), times(numContainers))
- .move(any(ContainerID.class), any(DatanodeDetails.class),
any(DatanodeDetails.class));
+ .move(any(ContainerID.class), any(DatanodeDetails.class),
any(DatanodeDetails.class));
Review Comment:
nit: let's keep original indentation
--
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]
