ChenSammi commented on pull request #1338: URL: https://github.com/apache/hadoop-ozone/pull/1338#issuecomment-693378960
Thanks @sodonnel for very thoughtful suggestions. > 2. Consider if a datanode was dead for some time, and is then restarted. It will start reporting a replica for the DELETED container and with the current logic I don't think we will clean it up as `isContainerEmpty(...)` will always return false due to it checking for only CLOSED containers. I feel that this should be handled in the ContainerReportHandler code. If a replica is reported for a container which is DELETED in SCM, then we should instruct the datanode to delete the replica. What do you think? The parameter hdds.scm.unknown-container.action currently controls what to do if a DN reports a containers which SCM does not know about. The default is to log a warning, but for a DELETED container, I think we should just delete the replica. Agree. we should just delete the replica in the case. > > 3. There is another corner case, where the dead node may have a container which is out of sync with the others, as it was dead when the blocks should have been removed. Therefore we had 3 empty container replicas, plus one non-empty dead one. The 3 empty replicas are removed, and the container moved to "DELETED". Then the dead node starts reporting a non-empty replica for the DELETED container. This is actually a wider problem, in that I believe there are scenarios where container replicas can get out of sync with one another and there is nothing to reconcile them and fix it. The extra stale replica is a real problem, not just in empty container deletion case, but also for non-empty CLOSED container case. After a delete block command is confirmed by 3 datanodes, this delete block command is finished and removed from block deletion log. So if after that, a fourth replica shows up, SCM will not send an new delete block command to this replica. But this fourth replica will have all remaining delete block commands executed if it's datanode is still on. Maybe we can rely on over replicated container replica deletion logic to remove this extra fourth replica. But it's hard to choose which one to delete if there deleteTransactionId and blockCommitSequenceId are different. Currently handleOverReplicatedContainer doesn't check these two fields of each replica. > > 4. In ContainerSafeModeRule, I think we need to consider DELETING and DELETED. If there are a lot of DELETED containers in the cluster, then they will never get replicas and the safemode rule will never get met. I think this could be as simple as excluding DELETING and DELETED when loading the containerMap (right now OPEN and CLOSING are excluded) so we don't wait for replicas on these containers. Got catch! > > 5. In `ReplicationManager.isContainerUnderReplicated(...)`, should we immediately return true if the containerState is DELETING or DELETED? Right. I also agree we should purge these deleted containers after a while. We can have a timer task to periodically remove the DELETED containers from SCM DB. What do you think? ---------------------------------------------------------------- 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
