[
https://issues.apache.org/jira/browse/HDDS-16456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16456:
------------------------------
Description:
h3. Finding
A non-atomic timeout re-arm on the write RPC tail resurrects a
recoveringContainerMap entry for a container that concurrently moved to
CLOSING/CLOSED; the scrub then marks that valid replica UNHEALTHY, driving SCM
re-replication of a good replica. Triggered by a concurrent write RPC and
CloseContainerCommand on a RECOVERING EC target; same bounded, self-healing
external effect as the stale-scrub demotion.
h3. Classification
* Verdict: REPRODUCED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding MC-2
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. An EC reconstruction target replica is created RECOVERING; {{addContainer}}
records a recovering-map entry (deadline = now + recovering timeout).
2. A delayed or retried PutBlock/WriteChunk RPC to that replica reaches the
tail of {{handlePutBlock}}/{{handleWriteChunk}}, which calls the private
{{KeyValueHandler.updateRecoveringContainerTimeout}}. That method reads the
container state ({{!= RECOVERING}} guard) and then calls
{{ContainerSet.updateRecoveringContainerTimeout}} (an unconditional
{{recoveringContainerMap.put}}), all without holding the container write lock.
3. Concurrently an SCM CloseContainerCommand drives
{{KeyValueHandler.markContainerForClose}}, which does hold the container write
lock: it removes the recovering-map entry and moves the container RECOVERING to
CLOSING.
4. The close executes fully between the write tail's state read and its put, so
the put re-inserts a stale ("zombie") recovering-map entry for a container that
is now CLOSING/CLOSED. The stale-recovering scrub later marks that valid
replica UNHEALTHY (a second route into the close-race), driving SCM to
re-replicate a good replica.
h3. Root cause
{{KeyValueHandler.updateRecoveringContainerTimeout}} performs a check-then-put
({{getContainerState()}} then
{{ContainerSet.updateRecoveringContainerTimeout}}) that is not atomic with
{{markContainerForClose}}. {{markContainerForClose}} takes the container write
lock while it removes the recovering-map entry and changes state, but the
write-tail update takes no lock, so the two can interleave and the put outlives
the close.
h3. Suggested fix
Attached patch [^HDDS-16456.001.patch] wraps the state check and the map update
in {{updateRecoveringContainerTimeout}} in the container write lock, mirroring
the lock discipline of {{markContainerForClose}}. The two operations are now
mutually exclusive: either the update runs first (puts the entry, which the
close then removes) or the close runs first (the update observes a
non-RECOVERING state and skips the put). Either way no stale entry survives. A
unit test drives a close holding the write lock and asserts the write-tail
update blocks on that lock and leaves no recovering-map entry once the
container is CLOSING.
Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
test, and review before merge.
was:
h3. Finding
A non-atomic timeout re-arm on the write RPC tail resurrects a
recoveringContainerMap entry for a container that concurrently moved to
CLOSING/CLOSED; the scrub then marks that valid replica UNHEALTHY, driving SCM
re-replication of a good replica. Triggered by a concurrent write RPC and
CloseContainerCommand on a RECOVERING EC target; same bounded, self-healing
external effect as the stale-scrub demotion.
h3. Classification
* Verdict: REPRODUCED
* Severity: High
* Source: Specula TLA+ model checking and confirmation debate, finding MC-2
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).
> Non atomic timeout re arm resurrects a recovering container entry and marks a
> valid EC replica UNHEALTHY
> --------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16456
> URL: https://issues.apache.org/jira/browse/HDDS-16456
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: HDDS-16456.001.patch
>
>
> h3. Finding
> A non-atomic timeout re-arm on the write RPC tail resurrects a
> recoveringContainerMap entry for a container that concurrently moved to
> CLOSING/CLOSED; the scrub then marks that valid replica UNHEALTHY, driving
> SCM re-replication of a good replica. Triggered by a concurrent write RPC and
> CloseContainerCommand on a RECOVERING EC target; same bounded, self-healing
> external effect as the stale-scrub demotion.
> h3. Classification
> * Verdict: REPRODUCED
> * Severity: High
> * Source: Specula TLA+ model checking and confirmation debate, finding MC-2
> 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. An EC reconstruction target replica is created RECOVERING;
> {{addContainer}} records a recovering-map entry (deadline = now + recovering
> timeout).
> 2. A delayed or retried PutBlock/WriteChunk RPC to that replica reaches the
> tail of {{handlePutBlock}}/{{handleWriteChunk}}, which calls the private
> {{KeyValueHandler.updateRecoveringContainerTimeout}}. That method reads the
> container state ({{!= RECOVERING}} guard) and then calls
> {{ContainerSet.updateRecoveringContainerTimeout}} (an unconditional
> {{recoveringContainerMap.put}}), all without holding the container write lock.
> 3. Concurrently an SCM CloseContainerCommand drives
> {{KeyValueHandler.markContainerForClose}}, which does hold the container
> write lock: it removes the recovering-map entry and moves the container
> RECOVERING to CLOSING.
> 4. The close executes fully between the write tail's state read and its put,
> so the put re-inserts a stale ("zombie") recovering-map entry for a container
> that is now CLOSING/CLOSED. The stale-recovering scrub later marks that valid
> replica UNHEALTHY (a second route into the close-race), driving SCM to
> re-replicate a good replica.
> h3. Root cause
> {{KeyValueHandler.updateRecoveringContainerTimeout}} performs a
> check-then-put ({{getContainerState()}} then
> {{ContainerSet.updateRecoveringContainerTimeout}}) that is not atomic with
> {{markContainerForClose}}. {{markContainerForClose}} takes the container
> write lock while it removes the recovering-map entry and changes state, but
> the write-tail update takes no lock, so the two can interleave and the put
> outlives the close.
> h3. Suggested fix
> Attached patch [^HDDS-16456.001.patch] wraps the state check and the map
> update in {{updateRecoveringContainerTimeout}} in the container write lock,
> mirroring the lock discipline of {{markContainerForClose}}. The two
> operations are now mutually exclusive: either the update runs first (puts the
> entry, which the close then removes) or the close runs first (the update
> observes a non-RECOVERING state and skips the put). Either way no stale entry
> survives. A unit test drives a close holding the write lock and asserts the
> write-tail update blocks on that lock and leaves no recovering-map entry once
> the container is CLOSING.
> 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]
