[GitHub] [ozone] siddhantsangwan commented on a diff in pull request #3831: HDDS-6893. EC: ReplicationManager - move the empty container handling into RM from Legacy

[GitBox <gi...@apache.org>]

siddhantsangwan commented on code in PR #3831:
URL: https://github.com/apache/ozone/pull/3831#discussion_r994188204


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -362,6 +364,35 @@ public void sendCloseContainerEvent(ContainerID containerID) {
     eventPublisher.fireEvent(SCMEvents.CLOSE_CONTAINER, containerID);
   }
 
+  /**
+   * Sends delete container command for the given container to the given
+   * datanode.
+   *
+   * @param container Container to be deleted
+   * @param replicaIndex Index of the container replica to be deleted
+   * @param datanode  The datanode on which the replica should be deleted
+   * @throws NotLeaderException when this SCM is not the leader
+   */
+  public void sendDeleteCommand(final ContainerInfo container, int replicaIndex,
+      final DatanodeDetails datanode) throws NotLeaderException {
+    LOG.info("Sending delete container command for container {}" +
+        " to datanode {}", container.containerID(), datanode);
+
+    final DeleteContainerCommand deleteCommand =
+        new DeleteContainerCommand(container.containerID(), false);
+    deleteCommand.setTerm(getScmTerm());
+
+    final CommandForDatanode<DeleteContainerCommandProto> datanodeCommand =
+        new CommandForDatanode<>(datanode.getUuid(), deleteCommand);
+    eventPublisher.fireEvent(SCMEvents.DATANODE_COMMAND, datanodeCommand);
+    containerReplicaPendingOps.scheduleDeleteReplica(container.containerID(),
+        datanode, replicaIndex);
+
+    metrics.incrNumDeletionCmdsSent();
+    metrics.incrNumDeletionBytesTotal(container.getUsedBytes());

Review Comment:
   Should `incrNumDeletionBytesTotal` only be called when the delete is complete?



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/protocol/commands/DeleteContainerCommand.java:
##########
@@ -56,6 +57,11 @@ public DeleteContainerCommand(long containerId, boolean forceFlag) {
     this.force = forceFlag;
   }
 
+  public DeleteContainerCommand(ContainerID containerID, boolean forceFlag) {
+    this.containerId = containerID.getId();

Review Comment:
   Since `getId()` is deprecated, we should change this later. I'll add a TODO.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org