This is an automated email from the ASF dual-hosted git repository.
swamirishi 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 c8bcf7f7d64 HDDS-13978. OMLockDetails should not be used as the object
returns a ThreadLocal Object (#9346)
c8bcf7f7d64 is described below
commit c8bcf7f7d64ea8b60ad38e43c00e128c34642cff
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)
---
.../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 bb8161f0fae..0330d4947f5 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
@@ -67,8 +67,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;
}
@@ -78,6 +80,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 c0580cdd16e..3868436eb51 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
@@ -18,6 +18,8 @@
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.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;
@@ -304,13 +306,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());
}
}
@@ -320,7 +329,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 42368e6bf4b..ac87efc6865 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;
@@ -189,6 +193,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]
