errose28 commented on code in PR #7293:
URL: https://github.com/apache/ozone/pull/7293#discussion_r1803567167
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java:
##########
@@ -143,12 +148,81 @@ public void markBlocksAsDeleted(KeyValueContainerData
data, Collection<Long> del
}
}
- public ContainerDiff diff(KeyValueContainerData thisContainer,
ContainerProtos.ContainerChecksumInfo otherInfo)
+ public ContainerDiff diff(KeyValueContainerData thisContainer,
ContainerProtos.ContainerChecksumInfo peerChecksumInfo)
throws IOException {
- // TODO HDDS-10928 compare the checksum info of the two containers and
return a summary.
- // Callers can act on this summary to repair their container replica
using the peer's replica.
- // This method will use the read lock, which is unused in the current
implementation.
- return new ContainerDiff();
+ Preconditions.assertNotNull(thisContainer, "Container data is null");
+ Optional<ContainerProtos.ContainerChecksumInfo.Builder>
thisContainerChecksumInfoBuilder =
+ read(thisContainer);
+ if (!thisContainerChecksumInfoBuilder.isPresent()) {
+ // TODO: To create containerMerkleTree or fail the request.
+ return null;
+ }
+
+ if (thisContainer.getContainerID() != peerChecksumInfo.getContainerID()) {
+ throw new IOException("Container Id does not match");
+ }
+
+ ContainerProtos.ContainerChecksumInfo thisChecksumInfo =
thisContainerChecksumInfoBuilder.get().build();
+
+ ContainerProtos.ContainerMerkleTree thisMerkleTree =
thisChecksumInfo.getContainerMerkleTree();
+ ContainerProtos.ContainerMerkleTree peerMerkleTree =
peerChecksumInfo.getContainerMerkleTree();
+
+ return compareContainerMerkleTree(thisMerkleTree, peerMerkleTree);
+ }
+
+ private ContainerDiff
compareContainerMerkleTree(ContainerProtos.ContainerMerkleTree thisMerkleTree,
+
ContainerProtos.ContainerMerkleTree peerMerkleTree) {
+
+ ContainerDiff report = new ContainerDiff();
+ if (thisMerkleTree.getDataChecksum() == peerMerkleTree.getDataChecksum()) {
+ return new ContainerDiff();
+ }
+
+ Map<Long, ContainerProtos.BlockMerkleTree> thisBlockMerkleTreeMap =
thisMerkleTree.getBlockMerkleTreeList()
+
.stream().collect(Collectors.toMap(ContainerProtos.BlockMerkleTree::getBlockID,
+ blockMerkleTree -> blockMerkleTree));
+
+ // Since we are reconciling our container with a peer, We only need to go
through the peer's block list
+ for (ContainerProtos.BlockMerkleTree peerBlockMerkleTree:
peerMerkleTree.getBlockMerkleTreeList()) {
+ // Check if our container has the peer block.
+ ContainerProtos.BlockMerkleTree thisBlockMerkleTree =
thisBlockMerkleTreeMap.get(
+ peerBlockMerkleTree.getBlockID());
+ if (thisBlockMerkleTree == null) {
+ report.addMissingBlock(peerBlockMerkleTree);
+ continue;
+ }
+
+ if (thisBlockMerkleTree.getBlockChecksum() !=
peerBlockMerkleTree.getBlockChecksum()) {
Review Comment:
Since we don't send the "bytes per checksum" info over as part of the tree,
I think the `unhealthy` flag is our best and only option. I think we actually
only need this in the `ChunkMerkleTree`. At the container and block level, even
if the peer's tree is unhealthy, ours may be unhealthy for a different reason
so we still need to walk down the tree.
--
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: issues-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
