errose28 commented on code in PR #9015:
URL: https://github.com/apache/ozone/pull/9015#discussion_r2383733013
##########
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:
Actually adding a max here will also break commutativity. The full merge
function should be as it is currently defined in this method so that trees
match regardless of the order nodes contact each other:
- If both blocks are live: Do a union of the chunks to compute the checksum.
- If one block is live and one is deleted, overwrite the live one with the
deleted one, using the deleted checksum
- If both blocks are deleted, use the largest checksum
Say we replace the second case with this:
- If one block is live and one is deleted, mark the live one as deleted, and
use the largest of the two checksums.
- This would also have to happen on the node with the deleted block: it
would update its deleted checksum to that of the live block if it is larger,
otherwise we have violated a + b = b + a
Here is a counter example to show this is not commutative: peer A has the
live block with checksum 1, peer B has the deleted block with checksum 2, and
peer C has a live block with checksum 3.
1. B reconciles with A: B is deleted with checksum 2
2. B reconciles with C: B is deleted with checksum 3
- B has reached its final state since it has talked to all nodes:
deleted with checksum 3
3. C reconciles with A: Chunk lists are merged giving C the new checksum 4
4. C reconciles with B: C is deleted but keeps its original larger checksum 4
- C has reached its final state since it has talked to all nodes:
deleted with checksum 4
Without even proceeding to A we can see the commutativity has been violated
since B and C cannot converge without another round. This merge function is
dependent on merges of live blocks happening before deleted blocks are
accounted for. There can be no dependence on order of operations in a
commutative function. This function fails if B never witnesses the union of A
and C.
So I believe the algorithm written currently is correct.
--
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]
