[
https://issues.apache.org/jira/browse/HDDS-16459?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16459:
------------------------------
Description:
h3. Finding
A non-atomic snapshot of (lastAppliedIndex, container2BCSIDMap) can persist a
BCSID from a later log index than the snapshot tag; on a crash that loses the
unsynced container RocksDB WAL while the fsync'd Ratis snapshot survives,
restart validation marks the healthy container spuriously UNHEALTHY. That
marking is durable across restart and reported to SCM, driving unnecessary
re-replication (bounded, recoverable external effect). ENV_LIMITED: the
end-to-end crash with selective WAL loss is not producible in the in-JVM
harness, and a control demonstrated that an atomic snapshot avoids the marking.
h3. Classification
* Verdict: ENV_LIMITED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding CR-2
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: dn-container-state-machine
Guidance:
campaigns/ozone-9fbf9ee/targets/020-dn-container-state-machine/.prompt-extra.md
{noformat}
{code:none}
specula run --agent=claude-code --effort=medium --keep-original
--max-parallel=2 \
--enable-reviews --confirm-debate --tlc-memory-limit=28G --tlc-worker-limit=8
\
"dn-container-state-machine|apache/ozone|Java|Use the target-specific
.prompt-extra.md"
{code}
Discovered under HDDS-16433 (Specula TLA+ verification effort). The TLA+
specification, counterexample, and confirmation debate live in the Specula run
artifacts.
Generated with Specula (Claude Opus 4.8).
h3. Summary
ContainerStateMachine.takeSnapshot writes the snapshot tag (term, index) and a
serialized copy of container2BCSIDMap, but the two are captured non-atomically.
container2BCSIDMap is updated eagerly during dispatch, ahead of the applied
index, so at the moment the snapshot is taken it can already hold a BCSID whose
owning Raft log entry is later than the snapshot index. Persisting that
ahead-of-tag BCSID makes the snapshot internally inconsistent.
h3. Steps to reproduce
1. A PutBlock at log index N+1 is dispatched and updates container2BCSIDMap[c]
= N+1 in memory, but the container data write to disk has not been fsynced yet.
2. takeSnapshot runs for applied index N. It serializes the whole
container2BCSIDMap, so the snapshot file records (tag=N, c -> N+1).
3. The datanode crashes, losing the unsynced container write for entry N+1,
while the snapshot file survives.
4. On restart, snapshot load seeds container2BCSIDMap[c] = N+1, but the on-disk
container is only at BCSID N (or the write is gone). Restart validation
compares recorded vs on-disk BCSID and marks the otherwise healthy container
UNHEALTHY, even though log replay over (N, logEnd] would have re-applied N+1
correctly.
This is difficult to force in a full-cluster test because it needs a crash
landing in the narrow window between the eager map update and the container
fsync, so it is verified here as a targeted invariant instead of an end-to-end
reproduction.
h3. Root cause
The snapshot tag and the persisted container2BCSIDMap are not a consistent cut.
The map reflects in-flight (dispatched but not-yet-applied, not-yet-durable)
writes, while the tag reflects only applied state. Any map entry whose BCSID
exceeds the snapshot index describes state that the snapshot is not allowed to
assert as durable.
h3. Suggested fix
Attached patch [^HDDS-16459.001.patch] filters the map at snapshot time to
exclude any container whose recorded BCSID exceeds the snapshot index, via a
new helper filterSnapshotBcsidMap(container2BCSIDMap, snapshotIndex). Only
BCSIDs at or below the tag are persisted, so the (tag, map) pair is a
consistent cut. This is safe because the recorded BCSID equals the Raft log
index of that container's last applied PutBlock: any excluded entry is
re-applied by Ratis log replay over (snapshotIndex, logEnd] after the snapshot
is loaded, so no committed data is lost, while a healthy container is no longer
spuriously marked UNHEALTHY after a crash in the write window. A unit test
asserts the invariant directly: entries at or below the tag are kept and an
entry from a later index is excluded, so no persisted BCSID ever exceeds the
snapshot tag.
Note: this closes the persisted-snapshot inconsistency. It does not add locking
between snapshotting and concurrent deleteContainer (an existing TODO in
persistContainerSet), which is a separate concern.
Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
test, and review before merge.
was:
h3. Finding
A non-atomic snapshot of (lastAppliedIndex, container2BCSIDMap) can persist a
BCSID from a later log index than the snapshot tag; on a crash that loses the
unsynced container RocksDB WAL while the fsync'd Ratis snapshot survives,
restart validation marks the healthy container spuriously UNHEALTHY. That
marking is durable across restart and reported to SCM, driving unnecessary
re-replication (bounded, recoverable external effect). ENV_LIMITED: the
end-to-end crash with selective WAL loss is not producible in the in-JVM
harness, and a control demonstrated that an atomic snapshot avoids the marking.
h3. Classification
* Verdict: ENV_LIMITED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding CR-2
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: dn-container-state-machine
Guidance:
campaigns/ozone-9fbf9ee/targets/020-dn-container-state-machine/.prompt-extra.md
{noformat}
{code:none}
specula run --agent=claude-code --effort=medium --keep-original
--max-parallel=2 \
--enable-reviews --confirm-debate --tlc-memory-limit=28G --tlc-worker-limit=8
\
"dn-container-state-machine|apache/ozone|Java|Use the target-specific
.prompt-extra.md"
{code}
Discovered under HDDS-16433 (Specula TLA+ verification effort). The TLA+
specification, counterexample, and confirmation debate live in the Specula run
artifacts.
Generated with Specula (Claude Opus 4.8).
> Non atomic container snapshot can mark a healthy container UNHEALTHY after
> crash with selective WAL loss
> --------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16459
> URL: https://issues.apache.org/jira/browse/HDDS-16459
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: HDDS-16459.001.patch
>
>
> h3. Finding
> A non-atomic snapshot of (lastAppliedIndex, container2BCSIDMap) can persist a
> BCSID from a later log index than the snapshot tag; on a crash that loses the
> unsynced container RocksDB WAL while the fsync'd Ratis snapshot survives,
> restart validation marks the healthy container spuriously UNHEALTHY. That
> marking is durable across restart and reported to SCM, driving unnecessary
> re-replication (bounded, recoverable external effect). ENV_LIMITED: the
> end-to-end crash with selective WAL loss is not producible in the in-JVM
> harness, and a control demonstrated that an atomic snapshot avoids the
> marking.
> h3. Classification
> * Verdict: ENV_LIMITED
> * Severity: High
> * Source: Specula TLA+ model checking and confirmation debate, finding CR-2
> h3. Reproduce
> {noformat}
> Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
> Specula: v1.1.0 (commit c6aa3dfa)
> Target: dn-container-state-machine
> Guidance:
> campaigns/ozone-9fbf9ee/targets/020-dn-container-state-machine/.prompt-extra.md
> {noformat}
> {code:none}
> specula run --agent=claude-code --effort=medium --keep-original
> --max-parallel=2 \
> --enable-reviews --confirm-debate --tlc-memory-limit=28G
> --tlc-worker-limit=8 \
> "dn-container-state-machine|apache/ozone|Java|Use the target-specific
> .prompt-extra.md"
> {code}
> Discovered under HDDS-16433 (Specula TLA+ verification effort). The TLA+
> specification, counterexample, and confirmation debate live in the Specula
> run artifacts.
> Generated with Specula (Claude Opus 4.8).
> h3. Summary
> ContainerStateMachine.takeSnapshot writes the snapshot tag (term, index) and
> a serialized copy of container2BCSIDMap, but the two are captured
> non-atomically. container2BCSIDMap is updated eagerly during dispatch, ahead
> of the applied index, so at the moment the snapshot is taken it can already
> hold a BCSID whose owning Raft log entry is later than the snapshot index.
> Persisting that ahead-of-tag BCSID makes the snapshot internally inconsistent.
> h3. Steps to reproduce
> 1. A PutBlock at log index N+1 is dispatched and updates
> container2BCSIDMap[c] = N+1 in memory, but the container data write to disk
> has not been fsynced yet.
> 2. takeSnapshot runs for applied index N. It serializes the whole
> container2BCSIDMap, so the snapshot file records (tag=N, c -> N+1).
> 3. The datanode crashes, losing the unsynced container write for entry N+1,
> while the snapshot file survives.
> 4. On restart, snapshot load seeds container2BCSIDMap[c] = N+1, but the
> on-disk container is only at BCSID N (or the write is gone). Restart
> validation compares recorded vs on-disk BCSID and marks the otherwise healthy
> container UNHEALTHY, even though log replay over (N, logEnd] would have
> re-applied N+1 correctly.
> This is difficult to force in a full-cluster test because it needs a crash
> landing in the narrow window between the eager map update and the container
> fsync, so it is verified here as a targeted invariant instead of an
> end-to-end reproduction.
> h3. Root cause
> The snapshot tag and the persisted container2BCSIDMap are not a consistent
> cut. The map reflects in-flight (dispatched but not-yet-applied,
> not-yet-durable) writes, while the tag reflects only applied state. Any map
> entry whose BCSID exceeds the snapshot index describes state that the
> snapshot is not allowed to assert as durable.
> h3. Suggested fix
> Attached patch [^HDDS-16459.001.patch] filters the map at snapshot time to
> exclude any container whose recorded BCSID exceeds the snapshot index, via a
> new helper filterSnapshotBcsidMap(container2BCSIDMap, snapshotIndex). Only
> BCSIDs at or below the tag are persisted, so the (tag, map) pair is a
> consistent cut. This is safe because the recorded BCSID equals the Raft log
> index of that container's last applied PutBlock: any excluded entry is
> re-applied by Ratis log replay over (snapshotIndex, logEnd] after the
> snapshot is loaded, so no committed data is lost, while a healthy container
> is no longer spuriously marked UNHEALTHY after a crash in the write window. A
> unit test asserts the invariant directly: entries at or below the tag are
> kept and an entry from a later index is excluded, so no persisted BCSID ever
> exceeds the snapshot tag.
> Note: this closes the persisted-snapshot inconsistency. It does not add
> locking between snapshotting and concurrent deleteContainer (an existing TODO
> in persistContainerSet), which is a separate concern.
> Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
> test, and review before merge.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
