[
https://issues.apache.org/jira/browse/HDDS-16453?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16453:
------------------------------
Description:
h3. Finding
An unguarded flush() during an apply commits data under a stale transaction
index (DurableIndexMatchesData violated), so a crash+recover re-applies an
already-durable entry. Reachable from the real notifyLeaderChanged /
saveConfiguration flush callers, but the consequence is masked by SCM's
idempotent-replay design (all @Replicate transactions are absolute-put/CAS; the
one non-idempotent quantity is a deprecated no-op). Internal invariant
violation with no external effect today but real downstream divergence risk the
moment a non-idempotent replicated transaction is added.
h3. Classification
* Verdict: MASKED
* Severity: Medium
* Source: Specula TLA+ model checking and confirmation debate, finding MC-1
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: scm-ha-tx-buffer
Guidance:
campaigns/ozone-9fbf9ee/targets/018-scm-ha-tx-buffer/.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
\
"scm-ha-tx-buffer|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-16432 (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
This is a latent (masked) defect. The invariant DurableIndexMatchesData is
violated in memory and on disk, but current SCM state converges on replay; a
full end-to-end divergence needs the unmasking condition below.
Deterministic site (saveConfiguration):
StatefulServiceStateManagerImpl.saveConfiguration runs inside the apply of its
own @Replicate call.
1. saveConfiguration stages the config bytes via addToBuffer, then calls the
unguarded SCMHADBTransactionBuffer.flush(). The outer applyTransaction has not
yet called updateLatestTrxInfo, so the durable index is still i-1.
2. flush() commits the batch atomically as {config data, index i-1}. The config
is durable under a stale index.
Concurrent site (notifyLeaderChanged): the leader-change callback calls the
unguarded flush() while an applyTransaction may be in flight (data staged for
index i, index not yet advanced), committing that transaction's data under
index i-1.
Unmasking condition (not reproducible in a plain unit test): a crash after such
a stale-index flush but before the index is persisted causes recovery to read
index i-1 and replay entry i, which is already durable. For every current SCM
transaction the replay is idempotent (absolute-put / CAS), so the state
converges and the harm is masked. If a non-idempotent replicated transaction
were added, the re-apply would diverge across SCM replicas with no automatic
recovery.
h3. Root cause
Two callers use the unguarded SCMHADBTransactionBuffer.flush(), which commits
the current batch under the current (not yet advanced) transaction index.
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/StatefulServiceStateManagerImpl.java
(saveConfiguration) and
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java
(notifyLeaderChanged). flush() takes only the write lock; unlike flushIfNeeded
it does not check applyingTransactions, so it is not deferred while a
transaction is being applied. HDDS-15065 added the applyingTransactions guard
and moved the periodic/monitor flush onto it, but these two direct-flush
callers were left unguarded.
h3. Suggested fix
Route both callers through the existing guarded flushIfNeeded(0) instead of
flush(). While a transaction apply is in progress it defers, so the staged data
is committed with a consistent index by the next flush; when no apply is in
progress it flushes pending data as before. This reuses the same guard
HDDS-15065 established. A unit test constructs StatefulServiceStateManagerImpl
with the real transaction buffer and asserts that saveConfiguration during an
apply window does not commit config under the stale index (the assertion fails
on the old flush() and passes with the fix); notifyLeaderChanged is the
concurrent variant fixed the same way. See [^HDDS-16453.001.patch].
Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
test, and review before merge.
was:
h3. Finding
An unguarded flush() during an apply commits data under a stale transaction
index (DurableIndexMatchesData violated), so a crash+recover re-applies an
already-durable entry. Reachable from the real notifyLeaderChanged /
saveConfiguration flush callers, but the consequence is masked by SCM's
idempotent-replay design (all @Replicate transactions are absolute-put/CAS; the
one non-idempotent quantity is a deprecated no-op). Internal invariant
violation with no external effect today but real downstream divergence risk the
moment a non-idempotent replicated transaction is added.
h3. Classification
* Verdict: MASKED
* Severity: Medium
* Source: Specula TLA+ model checking and confirmation debate, finding MC-1
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: scm-ha-tx-buffer
Guidance:
campaigns/ozone-9fbf9ee/targets/018-scm-ha-tx-buffer/.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
\
"scm-ha-tx-buffer|apache/ozone|Java|Use the target-specific .prompt-extra.md"
{code}
Discovered under HDDS-16432 (Specula TLA+ verification effort). The TLA+
specification, counterexample, and confirmation debate live in the Specula run
artifacts.
Generated with Specula (Claude Opus 4.8).
> Unguarded transaction buffer flush commits data under a stale index (masked
> by idempotent replay)
> -------------------------------------------------------------------------------------------------
>
> Key: HDDS-16453
> URL: https://issues.apache.org/jira/browse/HDDS-16453
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: HDDS-16453.001.patch
>
>
> h3. Finding
> An unguarded flush() during an apply commits data under a stale transaction
> index (DurableIndexMatchesData violated), so a crash+recover re-applies an
> already-durable entry. Reachable from the real notifyLeaderChanged /
> saveConfiguration flush callers, but the consequence is masked by SCM's
> idempotent-replay design (all @Replicate transactions are absolute-put/CAS;
> the one non-idempotent quantity is a deprecated no-op). Internal invariant
> violation with no external effect today but real downstream divergence risk
> the moment a non-idempotent replicated transaction is added.
> h3. Classification
> * Verdict: MASKED
> * Severity: Medium
> * Source: Specula TLA+ model checking and confirmation debate, finding MC-1
> h3. Reproduce
> {noformat}
> Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
> Specula: v1.1.0 (commit c6aa3dfa)
> Target: scm-ha-tx-buffer
> Guidance:
> campaigns/ozone-9fbf9ee/targets/018-scm-ha-tx-buffer/.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 \
> "scm-ha-tx-buffer|apache/ozone|Java|Use the target-specific
> .prompt-extra.md"
> {code}
> Discovered under HDDS-16432 (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
> This is a latent (masked) defect. The invariant DurableIndexMatchesData is
> violated in memory and on disk, but current SCM state converges on replay; a
> full end-to-end divergence needs the unmasking condition below.
> Deterministic site (saveConfiguration):
> StatefulServiceStateManagerImpl.saveConfiguration runs inside the apply of
> its own @Replicate call.
> 1. saveConfiguration stages the config bytes via addToBuffer, then calls the
> unguarded SCMHADBTransactionBuffer.flush(). The outer applyTransaction has
> not yet called updateLatestTrxInfo, so the durable index is still i-1.
> 2. flush() commits the batch atomically as {config data, index i-1}. The
> config is durable under a stale index.
> Concurrent site (notifyLeaderChanged): the leader-change callback calls the
> unguarded flush() while an applyTransaction may be in flight (data staged for
> index i, index not yet advanced), committing that transaction's data under
> index i-1.
> Unmasking condition (not reproducible in a plain unit test): a crash after
> such a stale-index flush but before the index is persisted causes recovery to
> read index i-1 and replay entry i, which is already durable. For every
> current SCM transaction the replay is idempotent (absolute-put / CAS), so the
> state converges and the harm is masked. If a non-idempotent replicated
> transaction were added, the re-apply would diverge across SCM replicas with
> no automatic recovery.
> h3. Root cause
> Two callers use the unguarded SCMHADBTransactionBuffer.flush(), which commits
> the current batch under the current (not yet advanced) transaction index.
> hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/StatefulServiceStateManagerImpl.java
> (saveConfiguration) and
> hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java
> (notifyLeaderChanged). flush() takes only the write lock; unlike
> flushIfNeeded it does not check applyingTransactions, so it is not deferred
> while a transaction is being applied. HDDS-15065 added the
> applyingTransactions guard and moved the periodic/monitor flush onto it, but
> these two direct-flush callers were left unguarded.
> h3. Suggested fix
> Route both callers through the existing guarded flushIfNeeded(0) instead of
> flush(). While a transaction apply is in progress it defers, so the staged
> data is committed with a consistent index by the next flush; when no apply is
> in progress it flushes pending data as before. This reuses the same guard
> HDDS-15065 established. A unit test constructs
> StatefulServiceStateManagerImpl with the real transaction buffer and asserts
> that saveConfiguration during an apply window does not commit config under
> the stale index (the assertion fails on the old flush() and passes with the
> fix); notifyLeaderChanged is the concurrent variant fixed the same way. See
> [^HDDS-16453.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]
