smengcl commented on code in PR #10496:
URL: https://github.com/apache/ozone/pull/10496#discussion_r3643172364
##########
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:
This fixes the detected interior-hole exit, but it still does not ensure
that `allChunksSuccessful` means the resulting block is complete.
`ContainerChecksumTreeManager.reportChunkIfHealthy()` can filter an unhealthy
trailing peer chunk before this method is called. If an earlier healthy chunk
is repaired, the loop finishes with `allChunksSuccessful == true` and advances
the BCSID although the trailing chunk remains absent.
It can be reproed with `ContainerChecksumTreeManager.diff()`: local BCSID 1
with chunk 0; peer BCSID 99 with chunks 0 and 1 healthy and trailing chunk 2
unhealthy. The diff returns only chunk 1, and reconciliation persists BCSID 99
without chunk 2.
Could we propagate block-completeness information from the diff, or
otherwise verify the resulting block before overwriting the BCSID, and cover
this with a diff-based regression test?
--
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]
