[jira] [Resolved] (HDDS-15542) Closed-container reconciliation advances BCSID past a hole, masking missing chunks

["Siyao Meng (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HDDS-15542?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Siyao Meng resolved HDDS-15542.
-------------------------------
    Fix Version/s: 2.3.0
       Resolution: Fixed

> Closed-container reconciliation advances BCSID past a hole, masking missing chunks
> ----------------------------------------------------------------------------------
>
>                 Key: HDDS-15542
>                 URL: https://issues.apache.org/jira/browse/HDDS-15542
>             Project: Apache Ozone
>          Issue Type: Bug
>          Components: Ozone Datanode
>            Reporter: Ritesh Shukla
>            Assignee: Ritesh Shukla
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 2.3.0
>
>
> h2. Summary
> When a closed container is reconciled against a peer, {{KeyValueHandler.reconcileChunksPerBlock}} pulls chunks from the peer to repair the local replica. The block commit sequence ID (BCSID) is a high-water mark: it is meant to assert that the replica holds, durably, every committed chunk up to that sequence id. The method's own contract says the BCSID is advanced to the peer's value only when the entire block is read and written successfully.
> The per-chunk loop can stop early in two ways:
> # A chunk read or write throws {{IOException}}. This path sets the loop's success flag to false, and the block is committed without overwriting the BCSID. Correct.
> # The loop reaches a chunk whose preceding chunk is missing locally (a hole). Writing it would leave a gap in the block file, which is not supported, so the loop breaks. This path does *not* clear the success flag.
> Because the success flag is initialized to true and the hole path leaves it untouched, the block is committed as if fully repaired: the BCSID is overwritten with the peer's higher value even though the chunks past the hole were never ingested, and the container BCSID is advanced to match. The replica then advertises a sequence id whose backing data it does not actually hold.
> h2. How the repair list develops a hole in normal operation
> The list of chunks handed to {{reconcileChunksPerBlock}} is not the peer's raw chunk list. It is the diff produced by {{ContainerChecksumTreeManager.compareChunkMerkleTrees}}, which adds a peer chunk to the repair list only through {{reportChunkIfHealthy}} (it is skipped if the peer reports it as unhealthy). A peer chunk is therefore left out of the repair list when:
> * the peer's chunk at that offset is corrupt (checksum mismatch). The scanner still records it in the merkle tree but marked unhealthy, and the diff skips it because reconciliation must not copy corrupt data; or
> * the peer's chunk at that offset is genuinely missing. The scanner omits a missing chunk from the merkle tree entirely ({{KeyValueContainerCheck}}: "Missing chunks should not be added to the merkle tree."), so the diff never sees that offset.
> Either way the repair list gets an interior gap. The bug then fires when the local replica is also missing the chunk before the gap: the loop ingests up to the gap, hits the hole break, and stops with chunks past the gap still absent. The precise condition is an interior offset where the local replica is missing the chunk and the peer's chunk is corrupt or missing, while the peer has a healthy chunk at a later offset that the local replica needs. That is a genuine multi-replica divergence (independent corruption or loss on different chunks across replicas), which is exactly the situation reconciliation exists to repair. It is an off-nominal but reachable state, not a pristine-cluster assumption, and the peer is not healthy: it has a bad chunk at the gap offset even though the chunks being pulled are healthy.
> h2. Impact
> SCM treats the reported BCSID as a freshness and completeness signal. {{AbstractContainerReportHandler}} raises the container's recorded sequence id to a healthy replica's reported value, and several SCM decisions key on that comparison: quasi-close to force-close, and the delete-versus-resurrect choice for DELETING/DELETED containers (a CLOSED or QUASI_CLOSED replica is deleted when its BCSID is at or below the container's). An inflated BCSID can therefore cause a holed replica that is missing committed data to be treated as the most up-to-date copy, and a complete replica to be deleted as redundant.
> This is a silent data-completeness defect, not a cosmetic counter drift. It is bounded to an off-nominal but reachable schedule: the peer must be ahead in BCSID, the repair list must contain an interior gap (as described above), the local replica must be missing the chunk before the gap, and a later SCM decision must act on the inflated value. It is not always-on.
> h2. Fix
> Treat the hole exit as a partial result, exactly like the {{IOException}} exit: clear the success flag before breaking out of the loop. The chunks ingested before the hole are still committed, but the BCSID is not advanced while the block remains incomplete. The post-reconciliation scanner rebuilds the merkle tree from what is actually on disk, and a later reconciliation round that first fills the missing chunk can then legitimately advance the BCSID.
> h2. Reproduction
> A unit test drives the real {{reconcileChunksPerBlock}} against a real closed container holding only the offset-0 chunk (BCSID 1) while a mocked peer advertises BCSID 99 and a repair list with a hole. Before the fix the block and container BCSID are advanced to 99; after the fix they stay at 1. The test is included in the linked pull request and serves as the regression guard.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org