You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by we...@apache.org on 2025/12/08 20:45:03 UTC
(ozone) 03/03: HDDS-13978. OMLockDetails should not be used as the object returns a ThreadLocal Object (#9346)
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 <sw...@gmail.com>
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: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org