errose28 commented on code in PR #7293:
URL: https://github.com/apache/ozone/pull/7293#discussion_r1803572976
##########
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()) {
+ compareBlockMerkleTree(report, thisBlockMerkleTree,
peerBlockMerkleTree);
+ }
+ }
+ return report;
+ }
+
+ private void compareBlockMerkleTree(ContainerDiff report,
ContainerProtos.BlockMerkleTree thisBlockMerkleTree,
+ ContainerProtos.BlockMerkleTree
peerBlockMerkleTree) {
+ Map<Long, ContainerProtos.ChunkMerkleTree> thisChunkMerkleTreeMap =
thisBlockMerkleTree.getChunkMerkleTreeList()
+
.stream().collect(Collectors.toMap(ContainerProtos.ChunkMerkleTree::getOffset,
+ chunkMerkleTree -> chunkMerkleTree));
+ List<ContainerProtos.ChunkMerkleTree> peerChunkMerkleTreeList =
peerBlockMerkleTree.getChunkMerkleTreeList();
+
+ // Since we are reconciling our container with a peer, We only need to go
through the peer's chunk list
+ for (ContainerProtos.ChunkMerkleTree peerChunkMerkleTree:
peerChunkMerkleTreeList) {
+ ContainerProtos.ChunkMerkleTree thisChunkMerkleTree =
thisChunkMerkleTreeMap.get(
+ peerChunkMerkleTree.getOffset());
+ if (thisChunkMerkleTree == null) {
+ report.addMissingChunk(peerChunkMerkleTree);
+ continue;
+ }
+
+ if (thisChunkMerkleTree.getChunkChecksum() !=
peerChunkMerkleTree.getChunkChecksum()) {
Review Comment:
One extra case accounting for the unhealthy flag: If both chunks are healthy
and the checksums don't match, we should throw an exception. This should never
happen because it means the checksums got corrupted among peers on the write
path but somehow they still match the data.
--
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]
