errose28 commented on code in PR #9015:
URL: https://github.com/apache/ozone/pull/9015#discussion_r2341671028
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeWriter.java:
##########
@@ -111,13 +110,82 @@ public void addBlock(long blockID) {
id2Block.computeIfAbsent(blockID, BlockMerkleTreeWriter::new);
}
+ /**
+ * Creates a deleted block entry in the merkle tree and assigns the block
this fixed checksum.
+ * If the block already exists with child data it is overwritten.
+ *
+ * This method is used on the reconciliation path to update the data
checksum used for a deleted block based on a
+ * peer's value.
+ */
+ public void setDeletedBlock(long blockID, long dataChecksum) {
+ BlockMerkleTreeWriter blockWriter = new BlockMerkleTreeWriter(blockID);
+ blockWriter.markDeleted(dataChecksum);
+ id2Block.put(blockID, blockWriter);
+ }
+
+ /**
+ * Merges the content from the provided tree with this tree writer.
+ * Conflicts where this tree writer and the incoming existingTree parameter
have an entry for the same block are
+ * resolved in the following manner:
+ * - A deleted block supersedes a live block
+ * - Data cannot be un-deleted, so if a delete is ever witnessed, that is
the state the block should converge to.
+ * - If both blocks are either deleted or live, the value in this writer
supersedes the value in the existingTree
+ * parameter.
+ * - Our writer has the last witnessed information that is going to be
persisted after this merge.
+ *
+ * For example, consider the case where a peer has deleted a block and we
have a corrupt copy that has not yet been
+ * deleted. When we reconcile with this peer, we will mark the block as
deleted and use the peer's checksum in our
+ * merkle tree to make the trees converge. The "fix" for corrupted data that
is supposed to be deleted is to delete
+ * it. After this, if the scanner runs again before the block is deleted, we
don't want to update the tree with the
+ * scanner's value because it would again diverge from the peer due to data
that is expected to be deleted.
+ * This would cause the checksum to oscillate back and forth until the block
is deleted, instead of converging.
+ */
+ public ContainerProtos.ContainerMerkleTree
update(ContainerProtos.ContainerMerkleTree existingTree) {
+ for (ContainerProtos.BlockMerkleTree existingBlockTree:
existingTree.getBlockMerkleTreeList()) {
+ long blockID = existingBlockTree.getBlockID();
+ BlockMerkleTreeWriter ourBlockTree = id2Block.get(blockID);
+ if (ourBlockTree != null) {
+ // both trees contain the block. We will only consider the
incoming/existing value if it does not match our
+ // current state
+ if (!ourBlockTree.isDeleted() && existingBlockTree.getDeleted()) {
+ setDeletedBlock(blockID, existingBlockTree.getDataChecksum());
+ }
+ // In all other cases, keep using our writer's value over the existing
one because either:
+ // - The deleted states match between the two blocks OR
+ // - Our block is deleted and the existing one is not, so we have the
latest value to use.
+ } else if (existingBlockTree.getDeleted()) {
+ // Our tree does not have this block. Only take the value if it is
deleted.
+ // The definitive set of live blocks will come from this tree writer.
+ setDeletedBlock(blockID, existingBlockTree.getDataChecksum());
+ }
+ }
+ return toProtoBuilder().build();
+ }
+
+ public ContainerProtos.ContainerMerkleTree
addDeletedBlocks(Collection<BlockData> blocks, boolean computeChecksum) {
+ for (BlockData block: blocks) {
+ long blockID = block.getLocalID();
+ BlockMerkleTreeWriter blockWriter = new BlockMerkleTreeWriter(blockID);
+ for (ContainerProtos.ChunkInfo chunkInfo: block.getChunks()) {
+ blockWriter.addChunks(new ChunkMerkleTreeWriter(chunkInfo, true));
+ }
+ blockWriter.markDeleted();
+ id2Block.put(blockID, blockWriter);
+ }
+ ContainerProtos.ContainerMerkleTree.Builder protoBuilder =
toProtoBuilder();
+ if (!computeChecksum) {
Review Comment:
I've now added javadoc to `ContainerChecksumTreeManager#addDeletedBlocks`
and `ContainerMerkleTreeWriter#addDeletedBlocks` to explain this.
--
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]
