HDDS-12572. Remove the ContainerID parameter when it has ContainerReplica.

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java

@@ -311,7 +311,7 @@ public void updateContainerReplica(final ContainerID cid,
                                      final ContainerReplica replica)
       throws ContainerNotFoundException {
     if (containerExist(cid)) {
-      containerStateManager.updateContainerReplica(cid, replica);
+      containerStateManager.updateContainerReplica(replica);
     } else {
       throwContainerNotFoundException(cid);
     }
@@ -322,7 +322,7 @@ public void removeContainerReplica(final ContainerID cid,
                                      final ContainerReplica replica)
       throws ContainerNotFoundException, ContainerReplicaNotFoundException {
     if (containerExist(cid)) {
-      containerStateManager.removeContainerReplica(cid, replica);
+      containerStateManager.removeContainerReplica(replica);
     } else {
       throwContainerNotFoundException(cid);
     }

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManager.java

@@ -145,14 +145,12 @@ public interface ContainerStateManager {
   /**
    *
    */
-  void updateContainerReplica(ContainerID id,
-                              ContainerReplica replica);
+  void updateContainerReplica(ContainerReplica replica);
 
   /**
    *
    */
-  void removeContainerReplica(ContainerID id,
-                              ContainerReplica replica);
+  void removeContainerReplica(ContainerReplica replica);
 
   /**
    *

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java

@@ -69,7 +69,6 @@
 import org.apache.hadoop.ozone.common.statemachine.InvalidStateTransitionException;
 import org.apache.hadoop.ozone.common.statemachine.StateMachine;
 import org.apache.ratis.util.AutoCloseableLock;
-import org.apache.ratis.util.Preconditions;
 import org.apache.ratis.util.function.CheckedConsumer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -416,8 +415,8 @@ public Set<ContainerReplica> getContainerReplicas(final ContainerID id) {
   }
 
   @Override
-  public void updateContainerReplica(final ContainerID id,
-                                     final ContainerReplica replica) {
+  public void updateContainerReplica(final ContainerReplica replica) {
+    final ContainerID id = replica.getContainerID();
     try (AutoCloseableLock ignored = writeLock(id)) {
       containers.updateContainerReplica(replica);
       // Clear any pending additions for this replica as we have now seen it.
@@ -427,10 +426,8 @@ public void updateContainerReplica(final ContainerID id,
   }
 
   @Override
-  public void removeContainerReplica(final ContainerID id,
-                                     final ContainerReplica replica) {
-    //TODO remove ContainerID parameter
-    Preconditions.assertEquals(id, replica.getContainerID(), "containerID");
+  public void removeContainerReplica(final ContainerReplica replica) {
+    final ContainerID id = replica.getContainerID();
     try (AutoCloseableLock ignored = writeLock(id)) {
       containers.removeContainerReplica(id, replica.getDatanodeDetails().getID());
       // Remove any pending delete replication operations for the deleted

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java

@@ -142,15 +142,13 @@ void setup() throws IOException, InvalidStateTransitionException {
 
     doAnswer(invocation -> {
       containerStateManager.updateContainerReplica(
-          ((ContainerID)invocation.getArguments()[0]),
           (ContainerReplica) invocation.getArguments()[1]);
       return null;
     }).when(containerManager).updateContainerReplica(
         any(ContainerID.class), any(ContainerReplica.class));
 
     doAnswer(invocation -> {
       containerStateManager.removeContainerReplica(
-          ((ContainerID)invocation.getArguments()[0]),
           (ContainerReplica) invocation.getArguments()[1]);
       return null;
     }).when(containerManager).removeContainerReplica(
@@ -265,8 +263,7 @@ public void testECReplicaIndexValidation() throws NodeNotFoundException,
     Map<DatanodeDetails, Integer> replicaMap = replicas.stream()
             .collect(Collectors.toMap(ContainerReplica::getDatanodeDetails,
                     ContainerReplica::getReplicaIndex));
-    replicas.forEach(r -> containerStateManager.updateContainerReplica(
-                    container.containerID(), r));
+    replicas.forEach(containerStateManager::updateContainerReplica);
     testReplicaIndexUpdate(container, datanodeOne, 0, replicaMap);
     testReplicaIndexUpdate(container, datanodeOne, 6, replicaMap);
     replicaMap.put(datanodeOne, 2);
@@ -300,14 +297,12 @@ public void testUnderReplicatedContainer()
     getReplicas(containerOne.containerID(),
         ContainerReplicaProto.State.CLOSED,
         datanodeOne, datanodeTwo, datanodeThree)
-        .forEach(r -> containerStateManager.updateContainerReplica(
-            containerOne.containerID(), r));
+        .forEach(containerStateManager::updateContainerReplica);
 
     getReplicas(containerTwo.containerID(),
         ContainerReplicaProto.State.CLOSED,
         datanodeOne, datanodeTwo, datanodeThree)
-        .forEach(r -> containerStateManager.updateContainerReplica(
-            containerTwo.containerID(), r));
+        .forEach(containerStateManager::updateContainerReplica);
 
 
     // SCM expects both containerOne and containerTwo to be in all the three
@@ -359,14 +354,12 @@ public void testOverReplicatedContainer() throws NodeNotFoundException,
     getReplicas(containerOne.containerID(),
         ContainerReplicaProto.State.CLOSED,
         datanodeOne, datanodeTwo, datanodeThree)
-        .forEach(r -> containerStateManager.updateContainerReplica(
-            containerOne.containerID(), r));
+        .forEach(containerStateManager::updateContainerReplica);
 
     getReplicas(containerTwo.containerID(),
         ContainerReplicaProto.State.CLOSED,
         datanodeOne, datanodeTwo, datanodeThree)
-        .forEach(r -> containerStateManager.updateContainerReplica(
-            containerTwo.containerID(), r));
+        .forEach(containerStateManager::updateContainerReplica);
 
 
     // SCM expects both containerOne and containerTwo to be in all the three
@@ -438,14 +431,8 @@ public void testClosingToClosed() throws NodeNotFoundException, IOException,
     containerStateManager.addContainer(containerOne.getProtobuf());
     containerStateManager.addContainer(containerTwo.getProtobuf());
 
-    containerOneReplicas.forEach(r ->
-        containerStateManager.updateContainerReplica(
-        containerTwo.containerID(), r));
-
-    containerTwoReplicas.forEach(r ->
-        containerStateManager.updateContainerReplica(
-        containerTwo.containerID(), r));
-
+    containerOneReplicas.forEach(containerStateManager::updateContainerReplica);
+    containerTwoReplicas.forEach(containerStateManager::updateContainerReplica);
 
     final ContainerReportsProto containerReport = getContainerReportsProto(
         containerOne.containerID(), ContainerReplicaProto.State.CLOSED,
@@ -655,7 +642,7 @@ private List<DatanodeDetails> setupECContainerForTesting(
             container.getSequenceId(),
             dns.toArray(new DatanodeDetails[0]));
     for (ContainerReplica r : replicas) {
-      containerStateManager.updateContainerReplica(container.containerID(), r);
+      containerStateManager.updateContainerReplica(r);
     }
 
     // Tell NodeManager that each DN hosts a replica of this container
@@ -725,14 +712,8 @@ public void testClosingToQuasiClosed()
     containerStateManager.addContainer(containerOne.getProtobuf());
     containerStateManager.addContainer(containerTwo.getProtobuf());
 
-    containerOneReplicas.forEach(r ->
-        containerStateManager.updateContainerReplica(
-        containerTwo.containerID(), r));
-
-    containerTwoReplicas.forEach(r ->
-        containerStateManager.updateContainerReplica(
-        containerTwo.containerID(), r));
-
+    containerOneReplicas.forEach(containerStateManager::updateContainerReplica);
+    containerTwoReplicas.forEach(containerStateManager::updateContainerReplica);
 
     final ContainerReportsProto containerReport = getContainerReportsProto(
         containerOne.containerID(), ContainerReplicaProto.State.QUASI_CLOSED,
@@ -795,14 +776,8 @@ public void testQuasiClosedToClosed()
     containerStateManager.addContainer(containerOne.getProtobuf());
     containerStateManager.addContainer(containerTwo.getProtobuf());
 
-    containerOneReplicas.forEach(r ->
-        containerStateManager.updateContainerReplica(
-        containerTwo.containerID(), r));
-
-    containerTwoReplicas.forEach(r ->
-        containerStateManager.updateContainerReplica(
-        containerTwo.containerID(), r));
-
+    containerOneReplicas.forEach(containerStateManager::updateContainerReplica);
+    containerTwoReplicas.forEach(containerStateManager::updateContainerReplica);
 
     final ContainerReportsProto containerReport = getContainerReportsProto(
         containerOne.containerID(), ContainerReplicaProto.State.CLOSED,

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManager.java

@@ -205,8 +205,7 @@ private void addReplica(ContainerInfo cont, DatanodeDetails node) {
         .setContainerState(ContainerReplicaProto.State.CLOSED)
         .setDatanodeDetails(node)
         .build();
-    containerStateManager
-        .updateContainerReplica(cont.containerID(), replica);
+    containerStateManager.updateContainerReplica(replica);
   }
 
   private ContainerInfo allocateContainer()

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestIncrementalContainerReportHandler.java

@@ -155,8 +155,7 @@ public void setup() throws IOException, InvalidStateTransitionException,
 
     doAnswer(invocation -> {
       containerStateManager
-          .removeContainerReplica(((ContainerID)invocation
-                  .getArguments()[0]),
+          .removeContainerReplica(
               (ContainerReplica)invocation.getArguments()[1]);
       return null;
     }).when(containerManager).removeContainerReplica(
@@ -175,8 +174,7 @@ public void setup() throws IOException, InvalidStateTransitionException,
 
     doAnswer(invocation -> {
       containerStateManager
-          .updateContainerReplica(((ContainerID)invocation
-                  .getArguments()[0]),
+          .updateContainerReplica(
               (ContainerReplica) invocation.getArguments()[1]);
       return null;
     }).when(containerManager).updateContainerReplica(
@@ -213,8 +211,7 @@ public void testClosingToClosed() throws IOException, TimeoutException {
         datanodeOne, datanodeTwo, datanodeThree);
 
     containerStateManager.addContainer(container.getProtobuf());
-    containerReplicas.forEach(r -> containerStateManager.updateContainerReplica(
-        container.containerID(), r));
+    containerReplicas.forEach(containerStateManager::updateContainerReplica);
 
     final IncrementalContainerReportProto containerReport =
         getIncrementalContainerReportProto(container.containerID(),
@@ -331,7 +328,7 @@ private List<DatanodeDetails> setupECContainerForTesting(
             container.getSequenceId(),
             dns.toArray(new DatanodeDetails[0]));
     for (ContainerReplica r : replicas) {
-      containerStateManager.updateContainerReplica(container.containerID(), r);
+      containerStateManager.updateContainerReplica(r);
     }
 
     // Tell NodeManager that each DN hosts a replica of this container
@@ -359,9 +356,7 @@ public void testClosingToQuasiClosed() throws IOException {
         datanodeOne, datanodeTwo, datanodeThree);
 
     containerStateManager.addContainer(container.getProtobuf());
-    containerReplicas.forEach(r -> containerStateManager.updateContainerReplica(
-        container.containerID(), r));
-
+    containerReplicas.forEach(containerStateManager::updateContainerReplica);
 
     final IncrementalContainerReportProto containerReport =
         getIncrementalContainerReportProto(container.containerID(),
@@ -396,8 +391,7 @@ public void testQuasiClosedToClosed() throws IOException {
         datanodeThree));
 
     containerStateManager.addContainer(container.getProtobuf());
-    containerReplicas.forEach(r -> containerStateManager.updateContainerReplica(
-        container.containerID(), r));
+    containerReplicas.forEach(containerStateManager::updateContainerReplica);
 
     final IncrementalContainerReportProto containerReport =
         getIncrementalContainerReportProto(container.containerID(),
@@ -440,8 +434,7 @@ public void testContainerStateTransitionToClosedWithMismatchingBCSID(LifeCycleSt
         datanodeThree));
 
     containerStateManager.addContainer(container.getProtobuf());
-    containerReplicas.forEach(r -> containerStateManager.updateContainerReplica(
-        container.containerID(), r));
+    containerReplicas.forEach(containerStateManager::updateContainerReplica);
 
     // Generate incremental container report with replica in CLOSED state with intentional lower bcsId
     final IncrementalContainerReportProto containerReport =
@@ -489,8 +482,7 @@ public void testOpenWithUnhealthyReplica() throws IOException {
         datanodeOne, datanodeTwo, datanodeThree);
 
     containerStateManager.addContainer(container.getProtobuf());
-    containerReplicas.forEach(r -> containerStateManager.updateContainerReplica(
-        container.containerID(), r));
+    containerReplicas.forEach(containerStateManager::updateContainerReplica);
 
     final IncrementalContainerReportProto containerReport =
         getIncrementalContainerReportProto(container.containerID(),
@@ -523,7 +515,7 @@ public void testDeleteContainer() throws IOException, TimeoutException,
 
     containerStateManager.addContainer(container.getProtobuf());
     containerReplicas.forEach(r -> {
-      containerStateManager.updateContainerReplica(container.containerID(), r);
+      containerStateManager.updateContainerReplica(r);
 
       assertDoesNotThrow(() -> nodeManager.addContainer(r.getDatanodeDetails(), container.containerID()),
           "Node should be found");
@@ -727,8 +719,7 @@ public void testECReplicaIndexValidation() throws NodeNotFoundException,
     Map<DatanodeDetails, Integer> replicaMap = replicas.stream()
             .collect(Collectors.toMap(ContainerReplica::getDatanodeDetails,
                     ContainerReplica::getReplicaIndex));
-    replicas.forEach(r -> containerStateManager.updateContainerReplica(
-            container.containerID(), r));
+    replicas.forEach(containerStateManager::updateContainerReplica);
     testReplicaIndexUpdate(container, dns.get(0), 0, replicaMap);
     testReplicaIndexUpdate(container, dns.get(0), 6, replicaMap);
     replicaMap.put(dns.get(0), 2);

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestContainerPlacement.java

@@ -260,6 +260,6 @@ private void addReplica(ContainerInfo cont, DatanodeDetails node) {
         .setDatanodeDetails(node)
         .build();
     containerManager.getContainerStateManager()
-        .updateContainerReplica(cont.containerID(), replica);
+        .updateContainerReplica(replica);
   }
 }

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManagerIntegration.java

@@ -349,43 +349,43 @@ public void testReplicaMap() throws Exception {
         .setContainerState(ContainerReplicaProto.State.OPEN)
         .setDatanodeDetails(dn2)
         .build();
-    containerStateManager.updateContainerReplica(id, replicaOne);
-    containerStateManager.updateContainerReplica(id, replicaTwo);
+    containerStateManager.updateContainerReplica(replicaOne);
+    containerStateManager.updateContainerReplica(replicaTwo);
     replicaSet = containerStateManager.getContainerReplicas(id);
     assertEquals(2, replicaSet.size());
     assertThat(replicaSet).contains(replicaOne);
     assertThat(replicaSet).contains(replicaTwo);
 
     // Test 3: Remove one replica node and then test
-    containerStateManager.removeContainerReplica(id, replicaOne);
+    containerStateManager.removeContainerReplica(replicaOne);
     replicaSet = containerStateManager.getContainerReplicas(id);
     assertEquals(1, replicaSet.size());
     assertThat(replicaSet).doesNotContain(replicaOne);
     assertThat(replicaSet).contains(replicaTwo);
 
     // Test 3: Remove second replica node and then test
-    containerStateManager.removeContainerReplica(id, replicaTwo);
+    containerStateManager.removeContainerReplica(replicaTwo);
     replicaSet = containerStateManager.getContainerReplicas(id);
     assertEquals(0, replicaSet.size());
     assertThat(replicaSet).doesNotContain(replicaOne);
     assertThat(replicaSet).doesNotContain(replicaTwo);
 
     // Test 4: Re-insert dn1
-    containerStateManager.updateContainerReplica(id, replicaOne);
+    containerStateManager.updateContainerReplica(replicaOne);
     replicaSet = containerStateManager.getContainerReplicas(id);
     assertEquals(1, replicaSet.size());
     assertThat(replicaSet).contains(replicaOne);
     assertThat(replicaSet).doesNotContain(replicaTwo);
 
     // Re-insert dn2
-    containerStateManager.updateContainerReplica(id, replicaTwo);
+    containerStateManager.updateContainerReplica(replicaTwo);
     replicaSet = containerStateManager.getContainerReplicas(id);
     assertEquals(2, replicaSet.size());
     assertThat(replicaSet).contains(replicaOne);
     assertThat(replicaSet).contains(replicaTwo);
 
     // Re-insert dn1
-    containerStateManager.updateContainerReplica(id, replicaOne);
+    containerStateManager.updateContainerReplica(replicaOne);
     replicaSet = containerStateManager.getContainerReplicas(id);
     assertEquals(2, replicaSet.size());
     assertThat(replicaSet).contains(replicaOne);

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java

@@ -673,7 +673,7 @@ public void testContainerDeleteWithInvalidKeyCount()
         .setEmpty(true)
         .build();
     // Update replica
-    containerStateManager.updateContainerReplica(containerId, replicaOne);
+    containerStateManager.updateContainerReplica(replicaOne);
 
     // Check replica updated with wrong keyCount
     scm.getContainerManager().getContainerReplicas(

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestDeletedBlocksTxnShell.java

@@ -43,7 +43,6 @@
 import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State;
 import org.apache.hadoop.hdds.scm.block.DeletedBlockLog;
 import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient;
-import org.apache.hadoop.hdds.scm.container.ContainerID;
 import org.apache.hadoop.hdds.scm.container.ContainerInfo;
 import org.apache.hadoop.hdds.scm.container.ContainerReplica;
 import org.apache.hadoop.hdds.scm.container.ContainerStateManager;
@@ -158,8 +157,7 @@ private void updateContainerMetadata(long cid) throws Exception {
         getContainerManager().getContainerStateManager();
     containerStateManager.addContainer(container.getProtobuf());
     for (ContainerReplica replica: replicaSet) {
-      containerStateManager.updateContainerReplica(
-          ContainerID.valueOf(cid), replica);
+      containerStateManager.updateContainerReplica(replica);
     }
   }