errose28 commented on code in PR #9015:
URL: https://github.com/apache/ozone/pull/9015#discussion_r2383665449
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -256,6 +172,38 @@ private void
compareContainerMerkleTree(ContainerProtos.ContainerChecksumInfo th
}
private void compareBlockMerkleTree(ContainerProtos.BlockMerkleTree
thisBlockMerkleTree,
+ ContainerProtos.BlockMerkleTree peerBlockMerkleTree, ContainerDiffReport
report) {
+
+ boolean thisBlockDeleted = thisBlockMerkleTree.getDeleted();
+ boolean peerBlockDeleted = peerBlockMerkleTree.getDeleted();
+
+ if (thisBlockDeleted) {
+ // Our block has been deleted.
+ if (peerBlockDeleted && thisBlockMerkleTree.getDataChecksum() <
peerBlockMerkleTree.getDataChecksum()) {
+ // If the peer's block is also deleted, use the largest checksum value
as the winner so that the values converge
+ // since there is no data corresponding to this block.
+ report.addDivergedDeletedBlock(peerBlockMerkleTree);
+ }
+ // Else, either the peer has not deleted the block or they have a lower
checksum for their deleted block.
+ // In these cases the peer needs to update their block.
+ // If the peer's block is deleted and its checksum matches ours, no
update is required.
+ } else {
+ if (peerBlockDeleted) {
+ // Our block has not yet been deleted, but peer's block has been.
+ // Mark our block as deleted to bring it in sync with the peer.
+ // Our block deleting service will eventually catch up.
+ // Our container scanner will not update this deleted block in the
merkle tree further even if it is still on
+ // disk so that we remain in sync with the peer.
+ // TODO HDDS-11765 Add support for deleting blocks from our replica
when a peer has already deleted the block.
+ report.addDivergedDeletedBlock(peerBlockMerkleTree);
Review Comment:
We should do a max check here as well.
We need to maintain the property that successful reconciliation between two
peers will make their checksums match. This means if one replica has the block
deleted and one has it live but the checksums are different, we need to do
something. Updating the live block's checksum without marking it deleted will
make it appear corrupted when viewed in isolation from other replicas, and
could be overwritten by the scanner.
This change uses a two-step rule to merge a live and deleted block,
regardless of how they show up:
- If one block is deleted and another is live, the live should be marked as
deleted since we can't un-delete data.
- Once both blocks are marked deleted, the largest checksum wins.
This way trees will converge without having to compare checksums of blocks
marked live and deleted which isn't very intuitive since only one has disk
state.
> if we do this Merkle Tree won't represent the current state of the system
It will represent the current state of live blocks on disk which is what
matters. Deleted blocks are no longer on the disk, we just track them to keep
the checksum from fluctuating and need an arbitrary merge function to make them
converge. Once any replica witnesses a delete, we know the block is no longer
referenced by live data in the OM.
--
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]
