This is an automated email from the ASF dual-hosted git repository.
smengcl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new bc21296f297 HDDS-16118. Release snapshot DB handles before
KeyDeletingService submits OM requests (#10987)
bc21296f297 is described below
commit bc21296f297dddc3207d052149ba14f9a44ba745
Author: Siyao Meng <[email protected]>
AuthorDate: Wed Aug 12 16:05:42 2026 -0700
HDDS-16118. Release snapshot DB handles before KeyDeletingService submits
OM requests (#10987)
---
.../ozone/om/service/KeyDeletingService.java | 15 +-
.../om/snapshot/filter/ReclaimableFilter.java | 15 +-
.../ozone/om/service/TestKeyDeletingService.java | 245 +++++++++++++++++++++
3 files changed, 271 insertions(+), 4 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
index e999a9e3ad8..8518a02b5cf 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
@@ -560,7 +560,8 @@ private
OzoneManagerProtocolProtos.SetSnapshotPropertyRequest getSetSnapshotRequ
* @param keyManager KeyManager of the underlying store.
*/
private void processDeletedKeysForStore(SnapshotInfo currentSnapshotInfo,
KeyManager keyManager,
- int remainNum) throws IOException, InterruptedException {
+ UncheckedAutoCloseableSupplier<OmSnapshot> currentSnapshot, int
remainNum)
+ throws IOException, InterruptedException {
String volume = null, bucket = null, snapshotTableKey = null;
if (currentSnapshotInfo != null) {
volume = currentSnapshotInfo.getVolumeName();
@@ -603,6 +604,16 @@ private void processDeletedKeysForStore(SnapshotInfo
currentSnapshotInfo, KeyMan
? keyManager.getPendingDeletionKeys(reclaimableKeyFilter,
remainNum)
: keyManager.getPendingDeletionKeys(volume, bucket, null,
reclaimableKeyFilter, remainNum);
Map<String, PurgedKey> purgedKeys =
pendingKeysDeletion.getPurgedKeys();
+
+ // Keep the snapshot GC locks until the OM requests below complete,
but release all snapshot DB read locks
+ // first. Otherwise the synchronous Ratis submission can wait for
the double buffer while the double buffer
+ // waits for a colliding snapshot DB write lock.
+ reclaimableKeyFilter.closeSnapshotDbHandles();
+ renameEntryFilter.closeSnapshotDbHandles();
+ if (currentSnapshot != null) {
+ currentSnapshot.close();
+ }
+
//submit purge requests if there are renamed entries to be purged or
keys to be purged.
if (!renamedTableEntries.isEmpty() || purgedKeys != null &&
!purgedKeys.isEmpty()) {
// Validating if the previous snapshot is still the same before
purging the blocks.
@@ -702,7 +713,7 @@ public BackgroundTaskResult call() {
snapInfo.getName())) {
KeyManager keyManager = snapInfo == null ?
getOzoneManager().getKeyManager()
: omSnapshot.get().getKeyManager();
- processDeletedKeysForStore(snapInfo, keyManager, remainNum);
+ processDeletedKeysForStore(snapInfo, keyManager, omSnapshot,
remainNum);
}
} catch (IOException e) {
LOG.error("Error while running delete files background task for
store {}. Will retry at next run.",
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java
index 76d2ddb4b8d..6d5e64b23d7 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java
@@ -216,10 +216,21 @@ public void close() throws IOException {
this.cleanup();
}
- private void cleanup() {
- this.snapshotIdLocks.releaseLock();
+ /**
+ * Closes snapshot DB handles while retaining the snapshot GC locks.
+ *
+ * The GC locks must remain held until the corresponding OM request is
+ * applied, but a synchronous request must not retain snapshot DB read locks
+ * since the double buffer may need a snapshot DB write lock to flush it.
+ */
+ public void closeSnapshotDbHandles() {
IOUtils.close(LOG, previousOmSnapshots);
previousOmSnapshots.clear();
+ }
+
+ private void cleanup() {
+ this.snapshotIdLocks.releaseLock();
+ closeSnapshotDbHandles();
previousSnapshotInfos.clear();
lockedSnapshotIds.clear();
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java
index 4c2efd47668..411cd65bebc 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java
@@ -20,6 +20,7 @@
import static
org.apache.hadoop.hdds.HddsConfigKeys.HDDS_CONTAINER_REPORT_INTERVAL;
import static
org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_INTERVAL;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_STRIPED_LOCK_SIZE_PREFIX;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL;
import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_DIR_DELETING_SERVICE_INTERVAL;
import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED;
@@ -30,6 +31,7 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
@@ -45,6 +47,7 @@
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableMap;
+import com.google.protobuf.ServiceException;
import java.io.File;
import java.io.IOException;
import java.io.UncheckedIOException;
@@ -56,8 +59,14 @@
import java.util.List;
import java.util.Map;
import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
@@ -69,6 +78,7 @@
import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.conf.StorageUnit;
+import org.apache.hadoop.hdds.scm.HddsWhiteboxTestUtils;
import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
import org.apache.hadoop.hdds.server.ServerUtils;
import org.apache.hadoop.hdds.utils.db.DBConfigFromFile;
@@ -103,6 +113,7 @@
import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.om.ratis.OzoneManagerDoubleBuffer;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
import org.apache.hadoop.ozone.om.snapshot.filter.ReclaimableKeyFilter;
@@ -110,6 +121,7 @@
import org.apache.ozone.test.GenericTestUtils;
import org.apache.ozone.test.OzoneTestBase;
import org.apache.ozone.test.tag.Flaky;
+import org.apache.ratis.protocol.ClientId;
import org.apache.ratis.util.ExitUtils;
import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier;
import org.junit.jupiter.api.AfterAll;
@@ -849,6 +861,239 @@ void testSnapshotExclusiveSize() throws Exception {
}
}
+ @Nested
+ @TestInstance(TestInstance.Lifecycle.PER_CLASS)
+ class SnapshotDbHandleLifecycle {
+
+ @BeforeAll
+ void setup(@TempDir File testDir) throws Exception {
+ scmBlockTestingClient = new ScmBlockLocationTestingClient(null, null, 0);
+ createConfig(testDir);
+ createSubject();
+ keyDeletingService.shutdown();
+ }
+
+ @AfterAll
+ void cleanup() {
+ if (om.stop()) {
+ om.join();
+ }
+ }
+
+ @Test
+ @DisplayName("KeyDeletingService should close the snapshot DB handle
before submitting an OM request")
+ void testSnapshotDbHandleClosedBeforeSubmit() throws Exception {
+ String volumeName = getTestName();
+ String bucketName = uniqueObjectName("bucket");
+ String snapshotName = uniqueObjectName("snap");
+ createVolumeAndBucket(volumeName, bucketName, false);
+ writeClient.createSnapshot(volumeName, bucketName, snapshotName);
+ om.awaitDoubleBufferFlush();
+
+ String snapshotKey = SnapshotInfo.getTableKey(volumeName, bucketName,
snapshotName);
+ SnapshotInfo snapshotInfo =
metadataManager.getSnapshotInfoTable().get(snapshotKey);
+ GenericTestUtils.waitFor(() -> {
+ try {
+ return
OmSnapshotManager.areSnapshotChangesFlushedToDB(metadataManager, snapshotInfo);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }, 100, 10000);
+ snapshotInfo.setDeepCleanedDeletedDir(true);
+ metadataManager.getSnapshotInfoTable().put(snapshotKey, snapshotInfo);
+
+ OzoneManager ozoneManager = Mockito.spy(om);
+ OmSnapshotManager omSnapshotManager =
Mockito.spy(om.getOmSnapshotManager());
+ when(ozoneManager.getOmSnapshotManager()).thenReturn(omSnapshotManager);
+
+ AtomicBoolean snapshotDbHandleClosed = new AtomicBoolean();
+ doAnswer(invocation -> {
+ UncheckedAutoCloseableSupplier<OmSnapshot> delegate =
+ (UncheckedAutoCloseableSupplier<OmSnapshot>)
invocation.callRealMethod();
+ return new UncheckedAutoCloseableSupplier<OmSnapshot>() {
+ @Override
+ public OmSnapshot get() {
+ return delegate.get();
+ }
+
+ @Override
+ public void close() {
+ delegate.close();
+ snapshotDbHandleClosed.set(true);
+ }
+ };
+ }).when(omSnapshotManager).getActiveSnapshot(volumeName, bucketName,
snapshotName);
+
+ AtomicBoolean requestSubmitted = new AtomicBoolean();
+ KeyDeletingService service = new KeyDeletingService(ozoneManager,
scmBlockTestingClient, 10000,
+ 100000, conf, 1, true) {
+ @Override
+ protected OzoneManagerProtocolProtos.OMResponse submitRequest(
+ OzoneManagerProtocolProtos.OMRequest omRequest) {
+ assertTrue(snapshotDbHandleClosed.get(), "Snapshot DB handle must be
closed before submitting to Ratis");
+ requestSubmitted.set(true);
+ return OzoneManagerProtocolProtos.OMResponse.newBuilder()
+ .setCmdType(omRequest.getCmdType())
+ .setStatus(OzoneManagerProtocolProtos.Status.OK)
+ .setSuccess(true)
+ .build();
+ }
+ };
+ service.shutdown();
+
+ service.new KeyDeletingTask(snapshotInfo.getSnapshotId()).call();
+
+ assertTrue(requestSubmitted.get());
+ }
+ }
+
+ /**
+ * Regression test for HDDS-16118: {@link KeyDeletingService} must not hold
a snapshot DB read lock
+ * while it submits a synchronous OM request. If it does, a snapshot purge
being applied by the
+ * OMDoubleBuffer flush thread blocks on the colliding snapshot DB write
lock, never releases its
+ * double-buffer permit, and the deep-clean submission then blocks forever
waiting for that permit.
+ *
+ * <p>The scenario is forced deterministically by shrinking two knobs: a
single double-buffer permit
+ * ({@code ozone.om.unflushed.transaction.max.count = 1}) and a single
snapshot DB lock stripe
+ * ({@code ozone.om.lock.stripes.snapshot_db_lock = 1}), so any snapshot
read lock collides with any
+ * snapshot write lock (the "different UUIDs, same stripe" case from the
fix).
+ */
+ @Nested
+ @TestInstance(TestInstance.Lifecycle.PER_CLASS)
+ class SnapshotDbHandlePermitDeadlock {
+
+ @BeforeAll
+ void setup(@TempDir File testDir) throws Exception {
+ scmBlockTestingClient = new ScmBlockLocationTestingClient(null, null, 0);
+ createConfig(testDir);
+ // A single double-buffer permit: one in-flight transaction exhausts the
semaphore, so a stalled
+ // flush immediately blocks the next transaction application.
+ conf.setInt(OMConfigKeys.OZONE_OM_UNFLUSHED_TRANSACTION_MAX_COUNT, 1);
+ // A single snapshot DB lock stripe: a read lock on one snapshot and a
write lock on another map
+ // to the same lock, guaranteeing the collision without hand-picking
colliding UUIDs.
+ conf.setInt(OZONE_MANAGER_STRIPED_LOCK_SIZE_PREFIX + "snapshot_db_lock",
1);
+ // Keep background deletion services out of the way; the test drives the
flow by hand.
+ conf.setTimeDuration(OZONE_BLOCK_DELETING_SERVICE_INTERVAL, 1,
TimeUnit.HOURS);
+ conf.setTimeDuration(OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL, 1,
TimeUnit.HOURS);
+ conf.setTimeDuration(OZONE_DIR_DELETING_SERVICE_INTERVAL, 1,
TimeUnit.HOURS);
+ createSubject();
+ keyDeletingService.shutdown();
+ }
+
+ @AfterAll
+ void cleanup() {
+ if (om.stop()) {
+ om.join();
+ }
+ }
+
+ @Test
+ @DisplayName("KeyDeletingService must not deadlock while a snapshot purge
holds the double-buffer permit")
+ void testNoPermitExhaustionDeadlock() throws Exception {
+ String volumeName = getTestName();
+ String bucketName = uniqueObjectName("bucket");
+ createVolumeAndBucket(volumeName, bucketName, false);
+
+ // deepCleanSnapshot: KeyDeletingService deep-cleans it and holds its
SNAPSHOT_DB read lock while
+ // it submits. purgeSnapshot: purged by the flush thread, which needs
the colliding write lock.
+ String deepCleanSnapshotName = uniqueObjectName("snap");
+ writeClient.createSnapshot(volumeName, bucketName,
deepCleanSnapshotName);
+ String purgeSnapshotName = uniqueObjectName("snap");
+ writeClient.createSnapshot(volumeName, bucketName, purgeSnapshotName);
+ om.awaitDoubleBufferFlush();
+
+ String deepCleanKey = SnapshotInfo.getTableKey(volumeName, bucketName,
deepCleanSnapshotName);
+ SnapshotInfo deepCleanInfo =
metadataManager.getSnapshotInfoTable().get(deepCleanKey);
+ GenericTestUtils.waitFor(() -> {
+ try {
+ return
OmSnapshotManager.areSnapshotChangesFlushedToDB(metadataManager, deepCleanInfo);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }, 100, 10000);
+ // Make the snapshot eligible for deleted-key deep cleaning so the task
reaches submitRequest.
+ deepCleanInfo.setDeepCleanedDeletedDir(true);
+ metadataManager.getSnapshotInfoTable().put(deepCleanKey, deepCleanInfo);
+
+ // Delete the purge snapshot so it can be purged. Background services
are suspended, so it is not
+ // auto-purged; the test submits the purge itself.
+ writeClient.deleteSnapshot(volumeName, bucketName, purgeSnapshotName);
+ om.awaitDoubleBufferFlush();
+ String purgeKey = SnapshotInfo.getTableKey(volumeName, bucketName,
purgeSnapshotName);
+ SnapshotInfo purgeInfo =
metadataManager.getSnapshotInfoTable().get(purgeKey);
+ assertNotNull(purgeInfo);
+
+ CountDownLatch kdsAtSubmit = new CountDownLatch(1);
+ CountDownLatch kdsMayProceed = new CountDownLatch(1);
+ AtomicBoolean paused = new AtomicBoolean();
+ KeyDeletingService service = new KeyDeletingService(om,
scmBlockTestingClient, 10000,
+ 100000, conf, 1, true) {
+ @Override
+ protected OzoneManagerProtocolProtos.OMResponse submitRequest(
+ OzoneManagerProtocolProtos.OMRequest omRequest) throws
ServiceException {
+ // Park on the first submission, while snapshot DB read locks
acquired during the metadata
+ // scan are (pre-fix) still held. Post-fix the fix has already
released them by now.
+ if (paused.compareAndSet(false, true)) {
+ kdsAtSubmit.countDown();
+ try {
+ kdsMayProceed.await();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new ServiceException(e);
+ }
+ }
+ return super.submitRequest(omRequest);
+ }
+ };
+ service.shutdown();
+
+ OzoneManagerDoubleBuffer doubleBuffer =
+
om.getOmRatisServer().getOmStateMachine().getOzoneManagerDoubleBuffer();
+ Semaphore permits = (Semaphore)
HddsWhiteboxTestUtils.getInternalState(doubleBuffer, "unFlushedTransactions");
+
+ ExecutorService executor = Executors.newFixedThreadPool(2);
+ try {
+ // 1. Run the deep-clean task. It parks in submitRequest holding the
snapshot DB read lock (pre-fix).
+ Future<?> kdsFuture = executor.submit(
+ () -> service.new
KeyDeletingTask(deepCleanInfo.getSnapshotId()).call());
+ assertTrue(kdsAtSubmit.await(30, TimeUnit.SECONDS), "KeyDeletingTask
never reached submitRequest");
+
+ // 2. Submit a snapshot purge. Applying it takes the single
double-buffer permit; on the pre-fix
+ // code its flush then blocks on the read lock still held above and
never releases the permit.
+ AtomicBoolean purgeDone = new AtomicBoolean();
+ OzoneManagerProtocolProtos.OMRequest purgeOmRequest =
OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.SnapshotPurge)
+
.setSnapshotPurgeRequest(OzoneManagerProtocolProtos.SnapshotPurgeRequest.newBuilder()
+ .addSnapshotDBKeys(purgeKey))
+ .setClientId(ClientId.randomId().toString())
+ .build();
+ Future<?> purgeFuture = executor.submit(() -> {
+ OzoneManagerRatisUtils.submitRequest(om, purgeOmRequest,
ClientId.randomId(), 1L);
+ purgeDone.set(true);
+ return null;
+ });
+ // Wait until the purge has taken the permit (pre-fix it stays taken)
or already completed (post-fix).
+ GenericTestUtils.waitFor(() -> permits.availablePermits() == 0 ||
purgeDone.get(), 50, 30000);
+
+ // 3. Release the deep-clean submission. It now needs the single
permit.
+ kdsMayProceed.countDown();
+
+ // Pre-fix: the permit is held by the stalled purge, so the task
blocks forever (deadlock).
+ // Post-fix: the purge released the permit, so the task completes.
+ try {
+ kdsFuture.get(60, TimeUnit.SECONDS);
+ } catch (TimeoutException e) {
+ fail("Permit-exhaustion deadlock reproduced: KeyDeletingService held
a snapshot DB read lock "
+ + "across its synchronous OM request while a snapshot purge held
the only double-buffer permit.");
+ }
+ purgeFuture.get(60, TimeUnit.SECONDS);
+ } finally {
+ kdsMayProceed.countDown();
+ executor.shutdownNow();
+ }
+ }
+ }
+
/**
* Tests failure scenarios.
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
