[
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: [email protected]
For additional commands, e-mail: [email protected]
