[
https://issues.apache.org/jira/browse/HDDS-16460?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16460:
------------------------------
Description:
h3. Finding
The container delete path leaves a zombie recoveringContainerMap entry (no
removeRecoveringContainer call). If the mask were absent the stale entry would
drive the pre-HDDS-7842 unguarded scrub to NPE; at HEAD the consequence is
prevented by the scrub null guard plus non-reused container IDs plus the
HDDS-15791 auto-create rejection. Reachable via the public force-delete API but
a hygiene/defensive gap only.
h3. Classification
* Verdict: MASKED
* Severity: Low
* Source: Specula TLA+ model checking and confirmation debate, finding MC-3
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: dn-ec-reconstruction
Guidance:
campaigns/ozone-9fbf9ee/targets/022-dn-ec-reconstruction/.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
\
"dn-ec-reconstruction|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. Steps to reproduce
1. EC reconstruction creates a target replica RECOVERING on a datanode;
{{ContainerSet.addContainer}} records a {{recoveringContainerMap}} entry.
2. Reconstruction is abandoned or SCM issues a (force) delete for that replica,
so {{KeyValueHandler.deleteContainer}} then {{deleteInternal}} runs
{{containerSet.removeContainer(containerId)}}.
3. The container leaves the {{ContainerSet}} but its {{recoveringContainerMap}}
entry lingers (a stale/zombie entry: container absent, map entry present).
Masked consequence (why this is a latent defect, not a live bug at HEAD): the
stale entry is later reaped by the scrub ({{getTasks}} removes it and the
{{getContainer(id) == null}} guard skips {{markContainerUnhealthy}}), and
container IDs are not reused, so no spurious UNHEALTHY results today. The guard
that prevents the crash was itself added to fix a NullPointerException the
scrub hit on exactly this stale-entry state (the scrub previously dereferenced
the container unconditionally), so the correspondence between the recovering
map and live containers is assumed by the code, not tolerated by design.
h3. Root cause
The two terminal exits from the RECOVERING state are asymmetric. The close path
({{KeyValueHandler.markContainerForClose}}) clears the recovering map via
{{containerSet.removeRecoveringContainer}}, but the delete path
({{deleteInternal}}) calls only {{containerSet.removeContainer}} and never
{{removeRecoveringContainer}}, leaving the entry behind.
h3. Suggested fix
Attached patch [^HDDS-16460.001.patch] adds
{{containerSet.removeRecoveringContainer(containerId)}} in {{deleteInternal}}
alongside {{removeContainer(containerId)}} (both run under the container write
lock), mirroring {{markContainerForClose}}, so both terminal RECOVERING exits
keep the recovering map consistent with live containers. A unit test
force-deletes a RECOVERING container through the public handler path and
asserts the recovering map is empty afterward.
Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
test, and review before merge.
was:
h3. Finding
The container delete path leaves a zombie recoveringContainerMap entry (no
removeRecoveringContainer call). If the mask were absent the stale entry would
drive the pre-HDDS-7842 unguarded scrub to NPE; at HEAD the consequence is
prevented by the scrub null guard plus non-reused container IDs plus the
HDDS-15791 auto-create rejection. Reachable via the public force-delete API but
a hygiene/defensive gap only.
h3. Classification
* Verdict: MASKED
* Severity: Low
* Source: Specula TLA+ model checking and confirmation debate, finding MC-3
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: dn-ec-reconstruction
Guidance:
campaigns/ozone-9fbf9ee/targets/022-dn-ec-reconstruction/.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
\
"dn-ec-reconstruction|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).
> Container delete leaves a zombie recoveringContainerMap entry (masked by
> scrub null guard)
> ------------------------------------------------------------------------------------------
>
> Key: HDDS-16460
> URL: https://issues.apache.org/jira/browse/HDDS-16460
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: HDDS-16460.001.patch
>
>
> h3. Finding
> The container delete path leaves a zombie recoveringContainerMap entry (no
> removeRecoveringContainer call). If the mask were absent the stale entry
> would drive the pre-HDDS-7842 unguarded scrub to NPE; at HEAD the consequence
> is prevented by the scrub null guard plus non-reused container IDs plus the
> HDDS-15791 auto-create rejection. Reachable via the public force-delete API
> but a hygiene/defensive gap only.
> h3. Classification
> * Verdict: MASKED
> * Severity: Low
> * Source: Specula TLA+ model checking and confirmation debate, finding MC-3
> h3. Reproduce
> {noformat}
> Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
> Specula: v1.1.0 (commit c6aa3dfa)
> Target: dn-ec-reconstruction
> Guidance:
> campaigns/ozone-9fbf9ee/targets/022-dn-ec-reconstruction/.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 \
> "dn-ec-reconstruction|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. Steps to reproduce
> 1. EC reconstruction creates a target replica RECOVERING on a datanode;
> {{ContainerSet.addContainer}} records a {{recoveringContainerMap}} entry.
> 2. Reconstruction is abandoned or SCM issues a (force) delete for that
> replica, so {{KeyValueHandler.deleteContainer}} then {{deleteInternal}} runs
> {{containerSet.removeContainer(containerId)}}.
> 3. The container leaves the {{ContainerSet}} but its
> {{recoveringContainerMap}} entry lingers (a stale/zombie entry: container
> absent, map entry present).
> Masked consequence (why this is a latent defect, not a live bug at HEAD): the
> stale entry is later reaped by the scrub ({{getTasks}} removes it and the
> {{getContainer(id) == null}} guard skips {{markContainerUnhealthy}}), and
> container IDs are not reused, so no spurious UNHEALTHY results today. The
> guard that prevents the crash was itself added to fix a NullPointerException
> the scrub hit on exactly this stale-entry state (the scrub previously
> dereferenced the container unconditionally), so the correspondence between
> the recovering map and live containers is assumed by the code, not tolerated
> by design.
> h3. Root cause
> The two terminal exits from the RECOVERING state are asymmetric. The close
> path ({{KeyValueHandler.markContainerForClose}}) clears the recovering map
> via {{containerSet.removeRecoveringContainer}}, but the delete path
> ({{deleteInternal}}) calls only {{containerSet.removeContainer}} and never
> {{removeRecoveringContainer}}, leaving the entry behind.
> h3. Suggested fix
> Attached patch [^HDDS-16460.001.patch] adds
> {{containerSet.removeRecoveringContainer(containerId)}} in {{deleteInternal}}
> alongside {{removeContainer(containerId)}} (both run under the container
> write lock), mirroring {{markContainerForClose}}, so both terminal RECOVERING
> exits keep the recovering map consistent with live containers. A unit test
> force-deletes a RECOVERING container through the public handler path and
> asserts the recovering map is empty afterward.
> 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]
