[
https://issues.apache.org/jira/browse/HDDS-16464?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16464:
------------------------------
Description:
h3. Finding
Internal invariant violation (DeepCleanedImpliesDrained) that demonstrably
propagates to an external effect: an unfenced deep-clean-flag write clobbers
the purge-driven reset, so the snapshot is skipped by every future
KeyDeletingService run and its now-reclaimable deleted keys are stranded for
the snapshot's entire retention (space leak, understated exclusive-size
accounting). Triggerable from ordinary key delete plus snapshot create/delete
GC activity; harm is bounded (no data loss, no premature reclaim) and only
escapes when the snapshot is itself deleted, so High rather than Critical.
h3. Classification
* Verdict: REPRODUCED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding MC-1
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: om-snapshot-gc
Guidance:
campaigns/ozone-9fbf9ee/targets/008-om-snapshot-gc/.prompt-extra.md
{noformat}
{code:none}
specula run --agent=claude-code --effort=high --keep-original --max-parallel=2 \
--enable-reviews --confirm-debate --tlc-memory-limit=28G --tlc-worker-limit=8
\
"om-snapshot-gc|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-15927 (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. Steps to reproduce
1. KeyDeletingService processes a snapshot's deleted keys. Before scanning it
captures the snapshot's previous snapshotId (expectedPreviousSnapshotId), which
determines which keys are reclaimable relative to the previous snapshot.
2. It reclaims the eligible keys via a PurgeKeys request, which is fenced: it
re-validates the previous snapshotId at apply time and skips if the chain
changed.
3. In the same run it also submits a SetSnapshotPropertyRequest with
deepCleanedDeletedKey=true to mark the snapshot fully cleaned. This request
carried no previous-snapshot fence.
4. Between the scan and the apply of the deep-clean flag, the snapshot chain
changes (a snapshot is created, deleted, or moved) so the snapshot's previous
is no longer the one seen at scan time. The purge is correctly skipped by its
fence, but the unfenced deep-clean flag is still applied.
5. The snapshot is now permanently marked deep cleaned while keys that should
have been reclaimed against the changed previous remain. Deep clean never runs
again for that snapshot, so those deleted keys are stranded and never reclaimed.
h3. Root cause
The key-deep-clean completion flag is written through
SetSnapshotPropertyRequest.deepCleanedDeletedKey, but that request had no
previous-snapshot fence, unlike the sibling PurgeKeys/PurgeDirectories requests
which carry expectedPreviousSnapshotID and re-validate it at apply time
(OMKeyPurgeRequest via SnapshotUtils.validatePreviousSnapshotId).
KeyDeletingService already computes expectedPreviousSnapshotId at scan time but
never propagated it to the deep-clean flag update, so the flag could be
committed against a stale view of the chain.
h3. Suggested fix
Fence the deep-clean flag the same way the purge path is fenced. Add an
optional NullableUUID expectedPreviousSnapshotID to SetSnapshotPropertyRequest
(OmClientProtocol.proto), populate it in KeyDeletingService when submitting
deepCleanedDeletedKey=true using the same expectedPreviousSnapshotId already
captured at scan time, and re-validate it at apply time in
OMSnapshotSetPropertyRequest.updateSnapshotProperty via
SnapshotUtils.validatePreviousSnapshotId before calling setDeepClean(true). On
a mismatch the request fails and is retried on the next run (fail-closed), so a
stale deep-clean flag is never committed. The change is backward compatible:
requests without the field (older senders) skip the check, matching the
existing purge-path convention. A request-level unit test builds a two-snapshot
chain and asserts that a deep-clean apply carrying a stale previous snapshotId
is rejected and leaves the flag unset, while a matching one sets it. See
[^HDDS-16464.001.patch].
Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
test, and review before merge.
was:
h3. Finding
Internal invariant violation (DeepCleanedImpliesDrained) that demonstrably
propagates to an external effect: an unfenced deep-clean-flag write clobbers
the purge-driven reset, so the snapshot is skipped by every future
KeyDeletingService run and its now-reclaimable deleted keys are stranded for
the snapshot's entire retention (space leak, understated exclusive-size
accounting). Triggerable from ordinary key delete plus snapshot create/delete
GC activity; harm is bounded (no data loss, no premature reclaim) and only
escapes when the snapshot is itself deleted, so High rather than Critical.
h3. Classification
* Verdict: REPRODUCED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding MC-1
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: om-snapshot-gc
Guidance:
campaigns/ozone-9fbf9ee/targets/008-om-snapshot-gc/.prompt-extra.md
{noformat}
{code:none}
specula run --agent=claude-code --effort=high --keep-original --max-parallel=2 \
--enable-reviews --confirm-debate --tlc-memory-limit=28G --tlc-worker-limit=8
\
"om-snapshot-gc|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-15927 (Specula TLA+ verification effort). The TLA+
specification, counterexample, and confirmation debate live in the Specula run
artifacts.
Generated with Specula (Claude Opus 4.8).
> Unfenced deep clean flag write strands a snapshot's reclaimable deleted keys
> for its retention
> ----------------------------------------------------------------------------------------------
>
> Key: HDDS-16464
> URL: https://issues.apache.org/jira/browse/HDDS-16464
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: HDDS-16464.001.patch
>
>
> h3. Finding
> Internal invariant violation (DeepCleanedImpliesDrained) that demonstrably
> propagates to an external effect: an unfenced deep-clean-flag write clobbers
> the purge-driven reset, so the snapshot is skipped by every future
> KeyDeletingService run and its now-reclaimable deleted keys are stranded for
> the snapshot's entire retention (space leak, understated exclusive-size
> accounting). Triggerable from ordinary key delete plus snapshot create/delete
> GC activity; harm is bounded (no data loss, no premature reclaim) and only
> escapes when the snapshot is itself deleted, so High rather than Critical.
> h3. Classification
> * Verdict: REPRODUCED
> * Severity: High
> * Source: Specula TLA+ model checking and confirmation debate, finding MC-1
> h3. Reproduce
> {noformat}
> Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
> Specula: v1.1.0 (commit c6aa3dfa)
> Target: om-snapshot-gc
> Guidance:
> campaigns/ozone-9fbf9ee/targets/008-om-snapshot-gc/.prompt-extra.md
> {noformat}
> {code:none}
> specula run --agent=claude-code --effort=high --keep-original
> --max-parallel=2 \
> --enable-reviews --confirm-debate --tlc-memory-limit=28G
> --tlc-worker-limit=8 \
> "om-snapshot-gc|apache/ozone|Java|Use the target-specific .prompt-extra.md"
> {code}
> Discovered under HDDS-15927 (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. Steps to reproduce
> 1. KeyDeletingService processes a snapshot's deleted keys. Before scanning it
> captures the snapshot's previous snapshotId (expectedPreviousSnapshotId),
> which determines which keys are reclaimable relative to the previous snapshot.
> 2. It reclaims the eligible keys via a PurgeKeys request, which is fenced: it
> re-validates the previous snapshotId at apply time and skips if the chain
> changed.
> 3. In the same run it also submits a SetSnapshotPropertyRequest with
> deepCleanedDeletedKey=true to mark the snapshot fully cleaned. This request
> carried no previous-snapshot fence.
> 4. Between the scan and the apply of the deep-clean flag, the snapshot chain
> changes (a snapshot is created, deleted, or moved) so the snapshot's previous
> is no longer the one seen at scan time. The purge is correctly skipped by its
> fence, but the unfenced deep-clean flag is still applied.
> 5. The snapshot is now permanently marked deep cleaned while keys that should
> have been reclaimed against the changed previous remain. Deep clean never
> runs again for that snapshot, so those deleted keys are stranded and never
> reclaimed.
> h3. Root cause
> The key-deep-clean completion flag is written through
> SetSnapshotPropertyRequest.deepCleanedDeletedKey, but that request had no
> previous-snapshot fence, unlike the sibling PurgeKeys/PurgeDirectories
> requests which carry expectedPreviousSnapshotID and re-validate it at apply
> time (OMKeyPurgeRequest via SnapshotUtils.validatePreviousSnapshotId).
> KeyDeletingService already computes expectedPreviousSnapshotId at scan time
> but never propagated it to the deep-clean flag update, so the flag could be
> committed against a stale view of the chain.
> h3. Suggested fix
> Fence the deep-clean flag the same way the purge path is fenced. Add an
> optional NullableUUID expectedPreviousSnapshotID to
> SetSnapshotPropertyRequest (OmClientProtocol.proto), populate it in
> KeyDeletingService when submitting deepCleanedDeletedKey=true using the same
> expectedPreviousSnapshotId already captured at scan time, and re-validate it
> at apply time in OMSnapshotSetPropertyRequest.updateSnapshotProperty via
> SnapshotUtils.validatePreviousSnapshotId before calling setDeepClean(true).
> On a mismatch the request fails and is retried on the next run (fail-closed),
> so a stale deep-clean flag is never committed. The change is backward
> compatible: requests without the field (older senders) skip the check,
> matching the existing purge-path convention. A request-level unit test builds
> a two-snapshot chain and asserts that a deep-clean apply carrying a stale
> previous snapshotId is rejected and leaves the flag unset, while a matching
> one sets it. See [^HDDS-16464.001.patch].
> 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]
