siddhantsangwan commented on code in PR #4399:
URL: https://github.com/apache/ozone/pull/4399#discussion_r1138173257
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestOverReplicatedProcessor.java:
##########
@@ -78,23 +71,17 @@ public void setup() {
}
@Test
- public void testDeleteContainerCommand() throws IOException {
+ public void testSuccessfulRun() throws IOException {
ContainerInfo container = ReplicationTestUtil
.createContainer(HddsProtos.LifeCycleState.CLOSED, repConfig);
queue.enqueue(new OverReplicatedHealthResult(
container, 3, false));
- Set<Pair<DatanodeDetails, SCMCommand<?>>> commands = new HashSet<>();
- DeleteContainerCommand cmd =
- new DeleteContainerCommand(container.getContainerID());
- cmd.setReplicaIndex(5);
- commands.add(Pair.of(MockDatanodeDetails.randomDatanodeDetails(), cmd));
- Mockito
- .when(replicationManager.processOverReplicatedContainer(any()))
- .thenReturn(commands);
+ Mockito.when(replicationManager.processOverReplicatedContainer(any()))
+ .thenReturn(1);
overReplicatedProcessor.processAll();
-
- Mockito.verify(replicationManager).sendDatanodeCommand(any(), any(),
any());
Review Comment:
Why remove these verifications in this test and the next one?
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestUnderReplicatedProcessor.java:
##########
@@ -156,8 +100,6 @@ public void testMessageRequeuedOnException() throws
IOException {
.thenThrow(new AssertionError("Should process only one item"));
underReplicatedProcessor.processAll();
- Mockito.verify(replicationManager, Mockito.times(0))
Review Comment:
Same question as above
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/RatisOverReplicationHandler.java:
##########
@@ -277,17 +282,18 @@ private Set<Pair<DatanodeDetails, SCMCommand<?>>>
createCommands(
// iterate through replicas in deterministic order
for (ContainerReplica replica : replicas) {
if (excess == 0) {
- return commands;
+ return commandsSent;
}
if (super.isPlacementStatusActuallyEqualAfterRemove(replicaSet, replica,
containerInfo.getReplicationFactor().getNumber())) {
- commands.add(Pair.of(replica.getDatanodeDetails(),
- createDeleteCommand(containerInfo)));
+ replicationManager.sendDeleteCommand(containerInfo,
+ replica.getReplicaIndex(), replica.getDatanodeDetails(), true);
+ commandsSent++;
excess--;
}
}
- return commands;
+ return commandsSent;
}
private DeleteContainerCommand createDeleteCommand(ContainerInfo container) {
Review Comment:
We can remove this method since it's unused now.
--
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]
