sodonnel commented on a change in pull request #1338: URL: https://github.com/apache/hadoop-ozone/pull/1338#discussion_r494180772
########## File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/AbstractContainerReportHandler.java ########## @@ -96,13 +101,24 @@ protected void processContainerReplica(final DatanodeDetails datanodeDetails, */ private void updateContainerStats(final DatanodeDetails datanodeDetails, final ContainerID containerId, - final ContainerReplicaProto replicaProto) + final ContainerReplicaProto replicaProto, + final EventPublisher publisher) throws ContainerNotFoundException { + final ContainerInfo containerInfo = containerManager + .getContainer(containerId); - if (isHealthy(replicaProto::getState)) { - final ContainerInfo containerInfo = containerManager - .getContainer(containerId); + if (containerInfo.getState() == HddsProtos.LifeCycleState.DELETED) { Review comment: It doesn't seem correct to put the logic to delete the replica for the DELETED container inside `updateContainerStats`. In `ContainerReportHandler#processContainerReplicas(..)` there is logic to delete an unknown container in the exception handler. Could we extract this into a new method, which is called from the exception handler. Then in `AbstractContainerReportHandler#updateContainerState(...)` handle the containers which should be deleted in the "case DELETED" branch of the swith statement. It could call that same extracted method - that way the logic to form the DeleteContainer command will be the same for both? It also seems more logical to put the delete inside UpdateContainerState rather than UpdateContainerStats. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org