This is an automated email from the ASF dual-hosted git repository. weichiu pushed a commit to branch ozone-2.1 in repository https://gitbox.apache.org/repos/asf/ozone.git
commit de24a1ad0c4402c4711852504c3fa121b8220461 Author: Swaminathan Balachandran <[email protected]> AuthorDate: Mon Nov 24 18:04:40 2025 -0500 HDDS-13978. OMLockDetails should not be used as the object returns a ThreadLocal Object (#9346) (cherry picked from commit c8bcf7f7d64ea8b60ad38e43c00e128c34642cff) --- .../ozone/om/snapshot/MultiSnapshotLocks.java | 6 +++++- .../hadoop/ozone/om/snapshot/SnapshotCache.java | 15 ++++++++++++--- .../ozone/om/snapshot/TestMultiSnapshotLocks.java | 21 ++++++++++++++++++++- .../hadoop/ozone/om/snapshot/TestSnapshotCache.java | 15 +++++++++++++++ 4 files changed, 52 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java index 52587730696..e353e104466 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java @@ -61,8 +61,10 @@ public synchronized OMLockDetails acquireLock(Collection<UUID> ids) throws OMExc lock.acquireReadLocks(resource, keys); if (omLockDetails.isLockAcquired()) { objectLocks.addAll(keys); + this.lockDetails = OMLockDetails.EMPTY_DETAILS_LOCK_ACQUIRED; + } else { + this.lockDetails = OMLockDetails.EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } - this.lockDetails = omLockDetails; return omLockDetails; } @@ -72,6 +74,8 @@ public synchronized void releaseLock() { } else { lockDetails = lock.releaseReadLocks(resource, this.objectLocks); } + this.lockDetails = lockDetails.isLockAcquired() ? OMLockDetails.EMPTY_DETAILS_LOCK_ACQUIRED : + OMLockDetails.EMPTY_DETAILS_LOCK_NOT_ACQUIRED; this.objectLocks.clear(); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java index ff0b04b0541..3da9b0cf344 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java @@ -19,6 +19,8 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND; import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_LOCK; +import static org.apache.hadoop.ozone.om.lock.OMLockDetails.EMPTY_DETAILS_LOCK_ACQUIRED; +import static org.apache.hadoop.ozone.om.lock.OMLockDetails.EMPTY_DETAILS_LOCK_NOT_ACQUIRED; import static org.apache.ozone.rocksdiff.RocksDBCheckpointDiffer.COLUMN_FAMILIES_TO_TRACK_IN_DAG; import com.google.common.annotations.VisibleForTesting; @@ -302,13 +304,20 @@ public UncheckedAutoCloseableSupplier<OMLockDetails> lock(UUID snapshotId) { () -> cleanup(snapshotId)); } + private OMLockDetails getEmptyOmLockDetails(OMLockDetails lockDetails) { + return lockDetails.isLockAcquired() ? EMPTY_DETAILS_LOCK_ACQUIRED : EMPTY_DETAILS_LOCK_NOT_ACQUIRED; + } + private UncheckedAutoCloseableSupplier<OMLockDetails> lock(Supplier<OMLockDetails> lockFunction, Supplier<OMLockDetails> unlockFunction, Supplier<Void> cleanupFunction) { - AtomicReference<OMLockDetails> lockDetails = new AtomicReference<>(lockFunction.get()); + Supplier<OMLockDetails> emptyLockFunction = () -> getEmptyOmLockDetails(lockFunction.get()); + Supplier<OMLockDetails> emptyUnlockFunction = () -> getEmptyOmLockDetails(unlockFunction.get()); + + AtomicReference<OMLockDetails> lockDetails = new AtomicReference<>(emptyLockFunction.get()); if (lockDetails.get().isLockAcquired()) { cleanupFunction.get(); if (!dbMap.isEmpty()) { - lockDetails.set(unlockFunction.get()); + lockDetails.set(emptyUnlockFunction.get()); } } @@ -318,7 +327,7 @@ private UncheckedAutoCloseableSupplier<OMLockDetails> lock(Supplier<OMLockDetail public void close() { lockDetails.updateAndGet((prevLock) -> { if (prevLock != null && prevLock.isLockAcquired()) { - return unlockFunction.get(); + return emptyUnlockFunction.get(); } return prevLock; }); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestMultiSnapshotLocks.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestMultiSnapshotLocks.java index 9c358a9261b..29f6a2d0efc 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestMultiSnapshotLocks.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestMultiSnapshotLocks.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.om.snapshot; +import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_GC_LOCK; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -31,9 +32,11 @@ import static org.mockito.Mockito.when; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.UUID; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; import org.apache.hadoop.ozone.om.lock.OMLockDetails; @@ -66,6 +69,21 @@ void setUp() { multiSnapshotLocks = new MultiSnapshotLocks(mockLock, mockResource, true); } + @Test + public void testMultiSnapshotLocksWithMultipleResourceLocksMultipleTimes() throws OMException { + OzoneManagerLock omLock = new OzoneManagerLock(new OzoneConfiguration()); + MultiSnapshotLocks multiSnapshotLocks1 = new MultiSnapshotLocks(omLock, SNAPSHOT_GC_LOCK, true); + MultiSnapshotLocks multiSnapshotLocks2 = new MultiSnapshotLocks(omLock, SNAPSHOT_GC_LOCK, true); + Collection<UUID> uuid1 = Collections.singleton(UUID.randomUUID()); + Collection<UUID> uuid2 = Collections.singleton(UUID.randomUUID()); + for (int i = 0; i < 10; i++) { + assertTrue(multiSnapshotLocks1.acquireLock(uuid1).isLockAcquired()); + assertTrue(multiSnapshotLocks2.acquireLock(uuid2).isLockAcquired()); + multiSnapshotLocks1.releaseLock(); + multiSnapshotLocks2.releaseLock(); + } + } + @Test void testAcquireLockSuccess() throws Exception { List<UUID> objects = Arrays.asList(obj1, obj2); @@ -107,7 +125,8 @@ void testReleaseLock() throws Exception { when(mockLock.acquireWriteLocks(eq(mockResource), anyCollection())).thenReturn(mockLockDetails); multiSnapshotLocks.acquireLock(objects); assertFalse(multiSnapshotLocks.getObjectLocks().isEmpty()); - + when(mockLockDetails.isLockAcquired()).thenReturn(false); + when(mockLock.releaseWriteLocks(eq(mockResource), anyCollection())).thenReturn(mockLockDetails); // Now release locks multiSnapshotLocks.releaseLock(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java index e3de9653f1f..3be27c9012a 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java @@ -18,12 +18,14 @@ package org.apache.hadoop.ozone.om.snapshot; import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.LeveledResource.VOLUME_LOCK; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +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.eq; @@ -43,6 +45,7 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; @@ -51,6 +54,7 @@ import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; import org.apache.hadoop.ozone.om.lock.OMLockDetails; import org.apache.hadoop.ozone.om.lock.OmReadOnlyLock; +import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; import org.apache.ozone.test.GenericTestUtils; import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier; import org.junit.jupiter.api.AfterEach; @@ -181,6 +185,17 @@ public void testLockHoldsWriteLock(int numberOfLocks) { verify(lock, times(numberOfLocks)).acquireResourceWriteLock(eq(SNAPSHOT_DB_LOCK)); } + @Test + public void testLockSupplierReturnsLockWithAnotherLockReleased() { + IOzoneManagerLock ozoneManagerLock = new OzoneManagerLock(new OzoneConfiguration()); + snapshotCache = new SnapshotCache(cacheLoader, CACHE_SIZE_LIMIT, omMetrics, 50, true, ozoneManagerLock); + try (UncheckedAutoCloseableSupplier<OMLockDetails> lockDetails = snapshotCache.lock()) { + ozoneManagerLock.acquireWriteLock(VOLUME_LOCK, "vol1"); + ozoneManagerLock.releaseWriteLock(VOLUME_LOCK, "vol1"); + assertTrue(lockDetails.get().isLockAcquired()); + } + } + @ParameterizedTest @ValueSource(ints = {0, 1, 5, 10}) @DisplayName("Tests lock(snapshotId) holds a write lock") --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
