errose28 commented on code in PR #9015:
URL: https://github.com/apache/ozone/pull/9015#discussion_r2383688333
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeWriter.java:
##########
@@ -111,13 +110,91 @@ 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();
+ }
+
+ /**
+ * Adds deleted blocks to this merkle tree. The blocks' checksums are
computed from the checksums in the BlockData.
+ * If a block with the same ID already exists in the tree, it is overwritten
as deleted with the checksum computed
+ * from the chunk checksums in the BlockData.
+ *
+ * The top level container data checksum is only computed in the returned
tree proto if computeChecksum is true.
+ * If it is false, the resulting tree proto will have data checksums for
each block, but an empty/unset data checksum
+ * for the container at the root of the tree.
+ */
+ public ContainerProtos.ContainerMerkleTree
addDeletedBlocks(Collection<BlockData> blocks, boolean computeChecksum) {
Review Comment:
Ideally yes, but this is complicated to implement. Currently the tree writer
tracks one copy of each block, and then once all are added it serializes
without conflicts. Now we would have to track multiple copies of deleted
blocks, and add conflict resolution at the time of serialization, since we
don't know the checksum of the incoming block until then. Also currently the
tree writer does not make decisions based on checksum values, that only happens
at higher layers.
Given this, I was ok leaving this gap for the uncommon case which would be
fixed with a single round of reconciliation. This won't cause the checksum to
fluctuate like the case where the scanner could update a block that the peer
deleted, because here we are moving the block to deleted which is terminal.
I've updated the javadoc to call this out.
--
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]
