[smengcl]

Generated-by: Codex (GPT-5.6 Sol)

What changes were proposed in this pull request?

DirectoryDeletingService (DDS) processes snapshots with parallel directory workers. The task thread retains the current snapshot DB handle, while each worker can retain previous-snapshot handles through reclaimable filters. Those handles hold striped SNAPSHOT_DB_LOCK read locks.

Before this change, a worker could synchronously submit PurgeDirectories while retaining its previous-snapshot handles, or DDS could submit its final snapshot-property request while retaining the task-owned current handle. If OMDoubleBufferFlushThread was applying an earlier snapshot purge that needed the colliding write-lock stripe and the unflushed transaction limit was reached, both sides could wait indefinitely.

This patch preserves per-snapshot worker parallelism while enforcing this order:

1. Each worker closes its previous-snapshot DB handles after it finishes reading.
2. DDS waits for every worker to reach that point, closes its current snapshot handle and iterator, then releases workers to submit requests.
3. Workers retain their snapshot-GC locks until their requests complete.

Snapshot task setup is serialized before the task-owned current handle is opened, preventing a second snapshot task from waiting while retaining such a handle. The worker barrier also handles a rejected executor submission during reconfiguration: DDS accounts for unscheduled workers, releases workers that already started, and retries the work in a later run.

The patch adds a deterministic circular-wait regression with one snapshot DB lock stripe and one unflushed transaction. It also verifies that snapshot work retains the configured worker parallelism and that a rejected worker submission does not strand an accepted worker.

What is the link to the Apache JIRA

HDDS-16164

How was this patch tested?

• mvn -pl :ozone-manager test -Dtest=TestDirectoryDeletingService -DskipShade -DskipRecon -DskipDocs (6 tests passed)
• mvn -pl :ozone-manager -am install -DskipTests -DskipShade -DskipRecon -DskipDocs (27-module reactor, 497 goals)
• mvn -pl :ozone-integration-test test -Dtest=TestDirectoryDeletingServiceWithFSO#testAOSKeyDeletingWithSnapshotCreateParallelExecution -DskipShade -DskipRecon -DskipDocs (1 test passed)
• mvn -pl :ozone-manager checkstyle:check -DskipTests -DskipShade -DskipRecon -DskipDocs (0 violations)
• git diff --check

[Copilot]

Pull request overview

This PR addresses a potential deadlock between DirectoryDeletingService (DDS) snapshot deep-clean processing and snapshot purge/double-buffer flushing by ensuring snapshot DB read locks (and task-owned snapshot handles/iterators) are released before DDS submits synchronous OM requests.

Changes:

• Add worker coordination so snapshot directory workers close prior-snapshot DB handles before submitting purge/set-snapshot-property requests, and DDS closes its task-owned current snapshot handle before releasing workers to submit.
• Serialize snapshot task setup using a lock to avoid waiting while retaining a task-owned snapshot DB handle.
• Add regression/unit tests covering the circular-wait scenario (single snapshot DB lock stripe + single unflushed transaction) and rejected worker submission handling; update existing tests/mocks for the new method signature.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java	Introduces snapshot-worker barriers to close snapshot DB handles before synchronous requests; adds snapshot setup serialization.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingService.java	Adds deterministic regression tests for the deadlock scenario and for rejected worker submission; updates existing calls for new APIs.
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingServiceWithFSO.java	Updates Mockito stubbing to account for the added Runnable parameter.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jojochuang]

Follow-up review on remaining deadlock classes after the snapshot deep-clean fix (not blocking).

[smengcl]

Addressed in 25146a2. Validation: 7 DirectoryDeletingService tests, 1 affected integration test, and Checkstyle across all 58 modules passed. Replies below explain the coordination changes and the AOS progress regression.

[jojochuang]

I pushed a proposed follow-up fix to my fork: 6138bd8 on HDDS-16164-dds-worker-submission-fix. The commit is based on the current PR head, 4428cf8, and can be cherry-picked.

The fix addresses the two outstanding worker-submission concerns:

• Submit directly to the DDS executor rather than falling back to the common pool. A stopped executor now reaches the existing rejection handling, avoiding the barrier deadlock when the configured worker count exceeds common-pool parallelism.
• Wait for the executor's completed-task count in the rejection test instead of assuming completion accounting has finished when the coordinator future returns.

The rejection test now covers both partial rejection and an already-stopped executor with three configured workers.

Validation on JDK 21: all 9 TestDirectoryDeletingService tests passed; both rejection cases also passed with JAVA_TOOL_OPTIONS=-Djava.util.concurrent.ForkJoinPool.common.parallelism=2; the repository checkstyle script and git diff --check passed. I did not rerun integration tests.

Generated-by: Codex

[SaketaChalamchala]

Thanks for the patch @smengcl. This change lgtm. Just a small follow up, could you add a tests for:

1. similar to KDS testSnapshotDbHandleClosedBeforeSubmit to assert that SNAPSHOT_DB locks are released before submitting purge requests?
2. A test where a deletion task worker throws (maybe in reclaimable filter) and assert that processDeletedDirsForStore does not hang and SNAPSHOT_DB locks are released.

Also, PTAL at the CI failures and the merge conflicts.

[smengcl]

Thanks @SaketaChalamchala . It is done.

[smengcl]

Thanks @jojochuang @rich7420 @SaketaChalamchala for the reviews.