kerneltime commented on code in PR #10496:
URL: https://github.com/apache/ozone/pull/10496#discussion_r3667692977
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java:
##########
@@ -1895,6 +1902,11 @@ private long reconcileChunksPerBlock(KeyValueContainer
container, Pipeline pipel
for (ContainerProtos.ChunkMerkleTree chunkMerkleTree : peerChunkList) {
long chunkOffset = chunkMerkleTree.getOffset();
if (!previousChunkPresent(blockID, chunkOffset, localOffset2Chunk)) {
+ // A hole remains: the chunk preceding this offset is missing
locally, so the block stays
+ // incomplete. Treat this like the per-chunk failure path below so
the commit does not
+ // overwrite the block/container BCSID with the peer's value while
data past the hole is
+ // absent. Advancing the BCSID here would falsely advertise
committed data we do not hold.
+ allChunksSuccessful = false;
Review Comment:
Thanks for the review @smengcl. Setting BCSID is always tricky, in a
scenario where there are only 2 replicas in the system left and post
reconciliation they have the same chunks (expect the trailing chunk), we have a
choice to make the BSCID the same or leave them as separate value. Arguably, in
the long run, BSCID should not play as decisive a role in the over all
algorithm. If a 3rd replica shows up with the same BCSID and a varying hash,
there must be a reconciliation look that must run. Do you want to deal with
this in a separate PR that focuses on BCSID matching algorithm overall?
--
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]
