[
https://issues.apache.org/jira/browse/HDDS-16438?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16438:
------------------------------
Description:
h3. Finding
Cache-first unordered enumeration breaks canSkipDir's descending-order premise,
so on crash/resume an unscanned FSO directory is skipped and its expired keys
are not collected; the demonstrated external effect (expired data left
uncollected past its lifecycle policy) is currently masked by the
next-generation fresh rescan, and absent that rescan the skip would persist and
recur under repeated faults. Reachable through the FSO lifecycle scan on any
bucket with a concurrent mkdir in the partial cache during a mid-scan crash.
MASKED: classified by the consequence the defect would expose if the rescan
mask were absent.
h3. Classification
* Verdict: MASKED
* 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-key-lifecycle
Guidance:
campaigns/ozone-9fbf9ee/targets/006-om-key-lifecycle/.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-key-lifecycle|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-16429 (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
# Enable a lifecycle policy on an FSO bucket. Under a directory, siblings dirA
and dirB exist in the directory table while dirC exists only in the partial
directory-table cache (for example a concurrent mkdir pending double-buffer
flush), and dirC sorts lower than dirA or between siblings.
# The lifecycle scan calls getSubDirectory, which lists cache children first
(hash-map order) then table children (ascending), producing an unordered
combined list, and pushes them onto the DFS LIFO stack in list order.
# Because a cache-resident child is pushed before the table children, it is
popped last, so siblings are not popped in strictly descending name order.
# The scan stops (crash, leader loss, suspend, or list-full abort) after a
higher-named sibling and persists lastScannedDir at that cursor while the
lower-named cache dir is still unscanned.
# On resume, canSkipDir compares paths lexicographically and skips any
directory whose path sorts greater than the cursor. The unscanned directory
that sorts greater than the cursor is wrongly skipped, so its keys are not
collected in that scan generation.
Masked: the next scan generation starts a fresh full scan from the root and
collects the stranded keys, so the effect is delayed expiration bounded to
roughly one generation, unless the reorder recurs every generation.
h3. Root cause
canSkipDir (KeyLifecycleService) decides a resumed directory was already
scanned when its path sorts strictly greater than the persisted lastScannedDir.
That is sound only if the DFS pops siblings in strictly descending name order,
which requires the collected child list to be sorted ascending before it is
pushed. getSubDirectory builds the list as cache children (ConcurrentHashMap
iteration order, unordered) followed by table children (ascending) and never
sorts the combined result, so the descending-pop-order premise is violated for
any directory sitting in the partial cache.
h3. Suggested fix
Sort the sub-directory list by name in ascending order at the end of
getSubDirectory (a new DirectoryList.sortByName that keeps the parallel key and
path lists index-aligned). This restores the strictly-descending pop order that
canSkipDir relies on, so a resumed directory is skipped only when it was
genuinely already scanned. See [^HDDS-16438.001.patch].
The added test pins the ordering invariant directly (DirectoryList sorts
siblings ascending and keeps the parallel lists aligned). A full end-to-end
reproduction requires a sub-directory that is resident only in the partial
cache and not yet in the table at scan time, which is not deterministically
injectable in the in-process test harness; the existing resume tests
(testDirectorySkippedAfterResume, testNestedFSODirectoryScanResume) continue to
pass with the fix.
Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
test, and review before merge.
was:
h3. Finding
Cache-first unordered enumeration breaks canSkipDir's descending-order premise,
so on crash/resume an unscanned FSO directory is skipped and its expired keys
are not collected; the demonstrated external effect (expired data left
uncollected past its lifecycle policy) is currently masked by the
next-generation fresh rescan, and absent that rescan the skip would persist and
recur under repeated faults. Reachable through the FSO lifecycle scan on any
bucket with a concurrent mkdir in the partial cache during a mid-scan crash.
MASKED: classified by the consequence the defect would expose if the rescan
mask were absent.
h3. Classification
* Verdict: MASKED
* 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-key-lifecycle
Guidance:
campaigns/ozone-9fbf9ee/targets/006-om-key-lifecycle/.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-key-lifecycle|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-16429 (Specula TLA+ verification effort). The TLA+
specification, counterexample, and confirmation debate live in the Specula run
artifacts.
Generated with Specula (Claude Opus 4.8).
> FSO lifecycle scan can skip an unscanned directory's expired keys after a mid
> scan crash
> ----------------------------------------------------------------------------------------
>
> Key: HDDS-16438
> URL: https://issues.apache.org/jira/browse/HDDS-16438
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: HDDS-16438.001.patch
>
>
> h3. Finding
> Cache-first unordered enumeration breaks canSkipDir's descending-order
> premise, so on crash/resume an unscanned FSO directory is skipped and its
> expired keys are not collected; the demonstrated external effect (expired
> data left uncollected past its lifecycle policy) is currently masked by the
> next-generation fresh rescan, and absent that rescan the skip would persist
> and recur under repeated faults. Reachable through the FSO lifecycle scan on
> any bucket with a concurrent mkdir in the partial cache during a mid-scan
> crash. MASKED: classified by the consequence the defect would expose if the
> rescan mask were absent.
> h3. Classification
> * Verdict: MASKED
> * 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-key-lifecycle
> Guidance:
> campaigns/ozone-9fbf9ee/targets/006-om-key-lifecycle/.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-key-lifecycle|apache/ozone|Java|Use the target-specific
> .prompt-extra.md"
> {code}
> Discovered under HDDS-16429 (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
> # Enable a lifecycle policy on an FSO bucket. Under a directory, siblings
> dirA and dirB exist in the directory table while dirC exists only in the
> partial directory-table cache (for example a concurrent mkdir pending
> double-buffer flush), and dirC sorts lower than dirA or between siblings.
> # The lifecycle scan calls getSubDirectory, which lists cache children first
> (hash-map order) then table children (ascending), producing an unordered
> combined list, and pushes them onto the DFS LIFO stack in list order.
> # Because a cache-resident child is pushed before the table children, it is
> popped last, so siblings are not popped in strictly descending name order.
> # The scan stops (crash, leader loss, suspend, or list-full abort) after a
> higher-named sibling and persists lastScannedDir at that cursor while the
> lower-named cache dir is still unscanned.
> # On resume, canSkipDir compares paths lexicographically and skips any
> directory whose path sorts greater than the cursor. The unscanned directory
> that sorts greater than the cursor is wrongly skipped, so its keys are not
> collected in that scan generation.
> Masked: the next scan generation starts a fresh full scan from the root and
> collects the stranded keys, so the effect is delayed expiration bounded to
> roughly one generation, unless the reorder recurs every generation.
> h3. Root cause
> canSkipDir (KeyLifecycleService) decides a resumed directory was already
> scanned when its path sorts strictly greater than the persisted
> lastScannedDir. That is sound only if the DFS pops siblings in strictly
> descending name order, which requires the collected child list to be sorted
> ascending before it is pushed. getSubDirectory builds the list as cache
> children (ConcurrentHashMap iteration order, unordered) followed by table
> children (ascending) and never sorts the combined result, so the
> descending-pop-order premise is violated for any directory sitting in the
> partial cache.
> h3. Suggested fix
> Sort the sub-directory list by name in ascending order at the end of
> getSubDirectory (a new DirectoryList.sortByName that keeps the parallel key
> and path lists index-aligned). This restores the strictly-descending pop
> order that canSkipDir relies on, so a resumed directory is skipped only when
> it was genuinely already scanned. See [^HDDS-16438.001.patch].
> The added test pins the ordering invariant directly (DirectoryList sorts
> siblings ascending and keeps the parallel lists aligned). A full end-to-end
> reproduction requires a sub-directory that is resident only in the partial
> cache and not yet in the table at scan time, which is not deterministically
> injectable in the in-process test harness; the existing resume tests
> (testDirectorySkippedAfterResume, testNestedFSODirectoryScanResume) continue
> to pass with the fix.
> 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]
