sodonnel commented on code in PR #4684:
URL: https://github.com/apache/ozone/pull/4684#discussion_r1188519579


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/ClosingContainerHandler.java:
##########
@@ -74,6 +79,37 @@ public boolean handle(ContainerCheckRequest request) {
             containerInfo, replica.getDatanodeDetails(), forceClose);
       }
     }
+
+    /*
+     * Empty containers in CLOSING state should be CLOSED.
+     *
+     * These are containers that are allocated in SCM but never got created
+     * on Datanodes. Since these containers don't have any replica associated
+     * with them, they are stuck in CLOSING state forever as there is no
+     * replicas to CLOSE.
+     *
+     * We should wait for sometime before moving the container to CLOSED state.
+     * This will give enough time for Datanodes to report the container,
+     * in cases where the container creation was successful on Datanodes.
+     *
+     * Should we have a separate configuration for this wait time?
+     * For now, we are using ReplicationManagerThread Interval * 5 as the wait
+     * time.
+     */
+
+
+    final Optional<ContainerInfo> emptyContainer = Optional.of(containerInfo)
+        .filter(c -> c.getReplicationType().equals(ReplicationType.RATIS))
+        .filter(c -> request.getContainerReplicas().isEmpty())
+        .filter(c -> c.getNumberOfKeys() == 0)
+        .filter(c -> {
+          long waitTime = replicationManager.getConfig().getInterval() * 5;
+          long closingTime = containerInfo.getStateEnterTime().toEpochMilli();
+          return (Time.now() - closingTime) > waitTime;

Review Comment:
   Rather than `Time.now()`, please call `replicationManager.getClock` and then 
get the time from the clock. Then in the tests we can use an instance of 
TestClock to manipulate the time and ensure the code does the correct thing.



-- 
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]

Reply via email to