You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2026/09/22 07:45:27 UTC
Re: [PR] HDDS-16164. DirectoryDeletingService could deadlock snapshot purge while retaining snapshot DB handles [ozone]
smengcl commented on code in PR #11009:
URL: https://github.com/apache/ozone/pull/11009#discussion_r4069448758
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -824,12 +920,25 @@ public BackgroundTaskResult call() {
} else if (!isPreviousPurgeTransactionFlushed()) {
return BackgroundTaskResult.EmptyTaskResult.newResult();
}
- try (UncheckedAutoCloseableSupplier<OmSnapshot> omSnapshot = snapInfo == null ? null :
- omSnapshotManager.getActiveSnapshot(snapInfo.getVolumeName(), snapInfo.getBucketName(),
- snapInfo.getName())) {
- KeyManager keyManager = snapInfo == null ? getOzoneManager().getKeyManager()
- : omSnapshot.get().getKeyManager();
- processDeletedDirsForStore(snapInfo, keyManager, run, pathLimitPerTask);
+ boolean snapshotProcessingLockAcquired = false;
+ try {
+ if (snapInfo != null) {
+ // Serialize snapshot setup before opening the snapshot DB. Workers still process each snapshot in
+ // parallel, but another snapshot task cannot wait here while retaining a task-owned DB handle.
+ snapshotProcessingLock.lockInterruptibly();
+ snapshotProcessingLockAcquired = true;
+ }
+ try (UncheckedAutoCloseableSupplier<OmSnapshot> omSnapshot = snapInfo == null ? null :
+ omSnapshotManager.getActiveSnapshot(snapInfo.getVolumeName(), snapInfo.getBucketName(),
+ snapInfo.getName())) {
+ KeyManager keyManager = snapInfo == null ? getOzoneManager().getKeyManager()
+ : omSnapshot.get().getKeyManager();
+ processDeletedDirsForStore(snapInfo, keyManager, omSnapshot, run, pathLimitPerTask);
+ }
+ } finally {
+ if (snapshotProcessingLockAcquired) {
+ snapshotProcessingLock.unlock();
+ }
}
Review Comment:
The lock covers the whole task, and the setup wording was misleading. Releasing it immediately after `getActiveSnapshot()` would allow another task to retain a DB handle while waiting for the shared worker pool. The revision replaces the lock with one BackgroundService coordinator for AOS and snapshot tasks. The separate deletion pool retains the configured worker parallelism.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
Review Comment:
Added the method summary and documented the coordinator requirement and current-snapshot handle ownership.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -639,7 +648,8 @@ private OzoneManagerProtocolProtos.SetSnapshotPropertyRequest getSetSnapshotRequ
* @param keyManager KeyManager of the underlying store.
*/
@VisibleForTesting
- void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyManager keyManager, long rnCnt, int remainNum)
+ void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyManager keyManager,
Review Comment:
Extracted worker scheduling, handle closure, and completion waiting into `runDeletionWorkers()`. `processDeletedDirsForStore()` now handles store setup and final snapshot-property updates. Rejection and interruption handling remain together in the coordination method.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -824,12 +920,25 @@ public BackgroundTaskResult call() {
} else if (!isPreviousPurgeTransactionFlushed()) {
return BackgroundTaskResult.EmptyTaskResult.newResult();
}
- try (UncheckedAutoCloseableSupplier<OmSnapshot> omSnapshot = snapInfo == null ? null :
- omSnapshotManager.getActiveSnapshot(snapInfo.getVolumeName(), snapInfo.getBucketName(),
- snapInfo.getName())) {
- KeyManager keyManager = snapInfo == null ? getOzoneManager().getKeyManager()
- : omSnapshot.get().getKeyManager();
- processDeletedDirsForStore(snapInfo, keyManager, run, pathLimitPerTask);
+ boolean snapshotProcessingLockAcquired = false;
Review Comment:
Removed the lock-acquisition flag and nested cleanup blocks. With one store coordinator, `call()` checks eligibility, opens the task-owned handle, and invokes store processing. The deletion-worker pool retains configured parallelism.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -658,43 +670,94 @@ void processDeletedDirsForStore(SnapshotInfo currentSnapshotInfo, KeyManager key
UUID expectedPreviousSnapshotId = currentSnapshotInfo == null ?
snapshotChainManager.getLatestGlobalSnapshotId() :
SnapshotUtils.getPreviousSnapshotId(currentSnapshotInfo, snapshotChainManager);
- Map<UUID, Pair<Long, Long>> exclusiveSizeMap = Maps.newConcurrentMap();
-
- CompletableFuture<Boolean> processedAllDeletedDirs = CompletableFuture.completedFuture(true);
final int parallelThreads = numberOfParallelThreadsPerStore.get();
+ CountDownLatch snapshotDbHandlesClosed = currentSnapshotInfo == null ? null :
Review Comment:
I added an AOS regression with three workers. One retains a snapshot DB read lock while another waits for unflushed transaction capacity. The scanning worker can still finish and release its handle, allowing both requests and the flush to complete. The diagram needs an additional dependency preventing worker B from finishing to form a deadlock. AOS therefore retains independent per-worker handle closure before submission.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -824,12 +920,25 @@ public BackgroundTaskResult call() {
} else if (!isPreviousPurgeTransactionFlushed()) {
return BackgroundTaskResult.EmptyTaskResult.newResult();
}
- try (UncheckedAutoCloseableSupplier<OmSnapshot> omSnapshot = snapInfo == null ? null :
- omSnapshotManager.getActiveSnapshot(snapInfo.getVolumeName(), snapInfo.getBucketName(),
- snapInfo.getName())) {
- KeyManager keyManager = snapInfo == null ? getOzoneManager().getKeyManager()
- : omSnapshot.get().getKeyManager();
- processDeletedDirsForStore(snapInfo, keyManager, run, pathLimitPerTask);
+ boolean snapshotProcessingLockAcquired = false;
Review Comment:
AOS did bypass the snapshot-task lock and share its deletion pool. The revision uses one coordinator for both, so another store cannot open snapshot handles until the current store’s workers finish. Workers within each store remain parallel. The regression checks that the next coordinator task stays queued during submission. The specific A/B/purge diagram still needs a dependency stopping B’s scan to establish deadlock.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org