[
https://issues.apache.org/jira/browse/HDDS-16444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16444:
------------------------------
Description:
h3. Finding
A concurrent cancelSnapshotDiff makes the diff task's catch-block
updateJobStatusToFailed throw IllegalStateException, so the genuine
storage-layer failure cause is never logged and a misleading exception escapes
the worker; the terminal job state stays correct (CANCELLED) and cleanup runs,
so the impact is diagnostic/robustness only on the operator log surface.
h3. Classification
* Verdict: REPRODUCED
* Severity: Low
* Source: Specula TLA+ model checking and confirmation debate, finding CR-3
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: om-snapshot-diff-jobs
Guidance:
campaigns/ozone-9fbf9ee/targets/009-om-snapshot-diff-jobs/.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-diff-jobs|apache/ozone|Java|Use the target-specific
.prompt-extra.md"
{code}
Discovered under HDDS-16430 (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. The snapshot diff executor runs generateSnapshotDiffReport for an
IN_PROGRESS job; a guard check passes.
2. A client calls cancelSnapshotDiff; updateJobStatus commits IN_PROGRESS to
CANCELLED on a separate thread.
3. The in-flight diff step throws a real checked exception (I/O or RocksDB
error during delta or report generation).
4. The catch block calls updateJobStatusToFailed first. The job status is now
CANCELLED, not IN_PROGRESS, so updateJobStatusToFailed throws
IllegalStateException.
5. That IllegalStateException is thrown before LOG.error(..., exception) and
before the wrap-and-rethrow, so the original failure cause is neither logged
nor propagated. The worker dies with a misleading IllegalStateException (cause
null) and the operator's log never records the genuine storage-layer fault.
The terminal job state remains correctly CANCELLED, so the impact is diagnostic
and robustness only.
h3. Root cause
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
updateJobStatusToFailed unconditionally threw IllegalStateException whenever
the current status was not IN_PROGRESS, and both catch blocks in
generateSnapshotDiffReport called it before logging the original exception. A
concurrent cancel that reaches CANCELLED first therefore turns a diff failure
into an unrelated IllegalStateException that hides the real cause.
h3. Suggested fix
Make updateJobStatusToFailed tolerate a concurrent terminal transition: if the
job is no longer IN_PROGRESS (or the row is gone), log a warning and return
instead of throwing, leaving the terminal state intact. Move LOG.error(...,
exception) ahead of the status update in both catch blocks so the true cause is
always recorded before any status change. A unit test drives the real
generateSnapshotDiffReport catch path with a concurrent cancel and asserts the
escaping exception wraps the original IOException, the cause is logged, and the
job stays CANCELLED. See [^HDDS-16444.001.patch].
Patch is an AI-drafted proposal (Specula + Claude); pending human build, full
test, and review before merge.
was:
h3. Finding
A concurrent cancelSnapshotDiff makes the diff task's catch-block
updateJobStatusToFailed throw IllegalStateException, so the genuine
storage-layer failure cause is never logged and a misleading exception escapes
the worker; the terminal job state stays correct (CANCELLED) and cleanup runs,
so the impact is diagnostic/robustness only on the operator log surface.
h3. Classification
* Verdict: REPRODUCED
* Severity: Low
* Source: Specula TLA+ model checking and confirmation debate, finding CR-3
h3. Reproduce
{noformat}
Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
Specula: v1.1.0 (commit c6aa3dfa)
Target: om-snapshot-diff-jobs
Guidance:
campaigns/ozone-9fbf9ee/targets/009-om-snapshot-diff-jobs/.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-diff-jobs|apache/ozone|Java|Use the target-specific
.prompt-extra.md"
{code}
Discovered under HDDS-16430 (Specula TLA+ verification effort). The TLA+
specification, counterexample, and confirmation debate live in the Specula run
artifacts.
Generated with Specula (Claude Opus 4.8).
> Concurrent cancelSnapshotDiff hides the real failure cause with an
> IllegalStateException
> ----------------------------------------------------------------------------------------
>
> Key: HDDS-16444
> URL: https://issues.apache.org/jira/browse/HDDS-16444
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: HDDS-16444.001.patch
>
>
> h3. Finding
> A concurrent cancelSnapshotDiff makes the diff task's catch-block
> updateJobStatusToFailed throw IllegalStateException, so the genuine
> storage-layer failure cause is never logged and a misleading exception
> escapes the worker; the terminal job state stays correct (CANCELLED) and
> cleanup runs, so the impact is diagnostic/robustness only on the operator log
> surface.
> h3. Classification
> * Verdict: REPRODUCED
> * Severity: Low
> * Source: Specula TLA+ model checking and confirmation debate, finding CR-3
> h3. Reproduce
> {noformat}
> Ozone commit: 9fbf9ee0cb1bd2f5f5d437b6719ebbe5309351fb
> Specula: v1.1.0 (commit c6aa3dfa)
> Target: om-snapshot-diff-jobs
> Guidance:
> campaigns/ozone-9fbf9ee/targets/009-om-snapshot-diff-jobs/.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-diff-jobs|apache/ozone|Java|Use the target-specific
> .prompt-extra.md"
> {code}
> Discovered under HDDS-16430 (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. The snapshot diff executor runs generateSnapshotDiffReport for an
> IN_PROGRESS job; a guard check passes.
> 2. A client calls cancelSnapshotDiff; updateJobStatus commits IN_PROGRESS to
> CANCELLED on a separate thread.
> 3. The in-flight diff step throws a real checked exception (I/O or RocksDB
> error during delta or report generation).
> 4. The catch block calls updateJobStatusToFailed first. The job status is now
> CANCELLED, not IN_PROGRESS, so updateJobStatusToFailed throws
> IllegalStateException.
> 5. That IllegalStateException is thrown before LOG.error(..., exception) and
> before the wrap-and-rethrow, so the original failure cause is neither logged
> nor propagated. The worker dies with a misleading IllegalStateException
> (cause null) and the operator's log never records the genuine storage-layer
> fault.
> The terminal job state remains correctly CANCELLED, so the impact is
> diagnostic and robustness only.
> h3. Root cause
> hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
> updateJobStatusToFailed unconditionally threw IllegalStateException whenever
> the current status was not IN_PROGRESS, and both catch blocks in
> generateSnapshotDiffReport called it before logging the original exception. A
> concurrent cancel that reaches CANCELLED first therefore turns a diff failure
> into an unrelated IllegalStateException that hides the real cause.
> h3. Suggested fix
> Make updateJobStatusToFailed tolerate a concurrent terminal transition: if
> the job is no longer IN_PROGRESS (or the row is gone), log a warning and
> return instead of throwing, leaving the terminal state intact. Move
> LOG.error(..., exception) ahead of the status update in both catch blocks so
> the true cause is always recorded before any status change. A unit test
> drives the real generateSnapshotDiffReport catch path with a concurrent
> cancel and asserts the escaping exception wraps the original IOException, the
> cause is logged, and the job stays CANCELLED. See [^HDDS-16444.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]
