sodonnel commented on code in PR #5324:
URL: https://github.com/apache/ozone/pull/5324#discussion_r1338261404
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -240,6 +249,80 @@ public void verifyAndFixupContainerData(ContainerData
containerData)
}
}
+ private void resolveDuplicate(KeyValueContainer existing,
+ KeyValueContainer toAdd) throws IOException {
+ if (existing.getContainerData().getReplicaIndex() != 0 ||
+ toAdd.getContainerData().getReplicaIndex() != 0) {
+ // This is an EC container. As EC Containers don't have a BSCID, we can't
+ // know which one has the most recent data. Additionally, it is possible
+ // for both copies to have a different replica index for the same
+ // container. Therefore we just let whatever one is loaded first win AND
+ // leave the other one on disk.
+ LOG.warn("Container {} is present at {} and at {}. Both are EC " +
+ "containers. Leaving both containers on disk.",
+ existing.getContainerData().getContainerID(),
+ existing.getContainerData().getContainerPath(),
+ toAdd.getContainerData().getContainerPath());
+ return;
+ }
+
+ long existingBCSID = existing.getBlockCommitSequenceId();
+ ContainerProtos.ContainerDataProto.State existingState
+ = existing.getContainerState();
+ long toAddBCSID = toAdd.getBlockCommitSequenceId();
+ ContainerProtos.ContainerDataProto.State toAddState
+ = toAdd.getContainerState();
+
+ if (existingState != toAddState) {
+ if (existingState == CLOSED) {
+ // If we have mis-matched states, always pick a closed one
+ LOG.warn("Container {} is present at {} with state CLOSED and at " +
+ "{} with state {}. Removing the latter container.",
+ existing.getContainerData().getContainerID(),
+ existing.getContainerData().getContainerPath(),
+ toAdd.getContainerData().getContainerPath(), toAddState);
+ KeyValueContainerUtil.removeContainer(toAdd.getContainerData(),
+ hddsVolume.getConf());
Review Comment:
RemoveContainer calls:
```
KeyValueContainerUtil.removeContainerDB(containerData, conf);
KeyValueContainerUtil.moveToDeletedContainerDir(containerData,
containerData.getVolume());
```
So it should result in it getting removed from disk.
I believe we should remove them, as otherwise what else is going to clean
them up, and there could be a significant number of containers and hence disk
space used.
For there to be 2 copies of a container on a single DN, one of the DNs
volumes originally containing the containers must have been marked failed for
some time. During that time, a new copy of the container was replicated from
another source to bring the redundancy from that failure back up to the
expected level. Then, we have had a DN restart bringing the old containers back
online.
So the redundancy should be fine - we have 2 copies available on this DN
too, so one should go. What is the value in keeping both? It will just request
in queries and confusion about why the DN is consuming more space than it
should, and if there are a lot of these containers, nobody is going to be able
to go through each manually to make a decision on each of them.
--
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]
