errose28 commented on code in PR #9015:
URL: https://github.com/apache/ozone/pull/9015#discussion_r2341577669
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -256,6 +173,62 @@ 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 && compareDataChecksums(thisBlockMerkleTree,
peerBlockMerkleTree) < 0) {
+ // 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.addDeletedBlock(peerBlockMerkleTree);
+ }
+ // Else, either the peer has not deleted the block or they have a lower
checksum for their deleted block.
+ // The peer needs to update their block.
+ } 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.
+ // TODO HDDS-11765 Add support for deleting blocks from our replica
when a peer has already deleted the block.
+ report.addDeletedBlock(peerBlockMerkleTree);
+ } else {
+ // Neither our nor peer's block is deleted. Walk the chunk list to
find differences.
+ compareChunkMerkleTrees(thisBlockMerkleTree, peerBlockMerkleTree,
report);
+ }
+ }
+ }
+
+ /**
+ * Compares the data checksums of two block merkle trees lexicographically
as big-endian binary strings.
+ */
+ private static int compareDataChecksums(ContainerProtos.BlockMerkleTree
tree1,
Review Comment:
That should work too. The comparison was done this way since the longs don't
actually represent numerical values, they are just a container for a 32 bit
string. The lexicographical comparison better aligns with this data type, but
it could be argued it's not worth the complexity.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -256,6 +173,62 @@ 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 && compareDataChecksums(thisBlockMerkleTree,
peerBlockMerkleTree) < 0) {
+ // 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.addDeletedBlock(peerBlockMerkleTree);
+ }
+ // Else, either the peer has not deleted the block or they have a lower
checksum for their deleted block.
+ // The peer needs to update their block.
+ } 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.
+ // TODO HDDS-11765 Add support for deleting blocks from our replica
when a peer has already deleted the block.
+ report.addDeletedBlock(peerBlockMerkleTree);
+ } else {
+ // Neither our nor peer's block is deleted. Walk the chunk list to
find differences.
+ compareChunkMerkleTrees(thisBlockMerkleTree, peerBlockMerkleTree,
report);
+ }
+ }
+ }
+
+ /**
+ * Compares the data checksums of two block merkle trees lexicographically
as big-endian binary strings.
+ */
+ private static int compareDataChecksums(ContainerProtos.BlockMerkleTree
tree1,
Review Comment:
Sure I can switch it to long comparison.
--
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]
