diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/FlatResource.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/FlatResource.java index f4d7e72ece3e..45534197866d 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/FlatResource.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/FlatResource.java @@ -28,7 +28,10 @@ public enum FlatResource implements Resource { // Lock acquired on a Snapshot's RocksDB Handle. SNAPSHOT_DB_LOCK("SNAPSHOT_DB_LOCK"), // Lock acquired on a Snapshot's Local Data. - SNAPSHOT_LOCAL_DATA_LOCK("SNAPSHOT_LOCAL_DATA_LOCK"); + SNAPSHOT_LOCAL_DATA_LOCK("SNAPSHOT_LOCAL_DATA_LOCK"), + // Lock acquired on a Snapshot's RocksDB contents. + SNAPSHOT_DB_CONTENT_LOCK("SNAPSHOT_DB_CONTENT_LOCK"); + private String name; private IOzoneManagerLock.ResourceManager resourceManager; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotDefragService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotDefragService.java index 212953cd874c..b99bb973931a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotDefragService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotDefragService.java @@ -93,7 +93,7 @@ public SnapshotDefragService(long interval, TimeUnit unit, long serviceTimeout, snapshotsDefraggedCount = new AtomicLong(0); running = new AtomicBoolean(false); IOzoneManagerLock omLock = ozoneManager.getMetadataManager().getLock(); - this.snapshotIdLocks = new MultiSnapshotLocks(omLock, SNAPSHOT_GC_LOCK, true); + this.snapshotIdLocks = new MultiSnapshotLocks(omLock, SNAPSHOT_GC_LOCK, true, 1); } @Override diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMDirectoriesPurgeResponseWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMDirectoriesPurgeResponseWithFSO.java index 1cf078bca0a8..1beddd253130 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMDirectoriesPurgeResponseWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMDirectoriesPurgeResponseWithFSO.java @@ -22,12 +22,14 @@ import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.SNAPSHOT_INFO_TABLE; +import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_CONTENT_LOCK; import com.google.common.annotations.VisibleForTesting; import jakarta.annotation.Nonnull; import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.UUID; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.hdds.utils.db.DBStore; @@ -36,11 +38,14 @@ import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; import org.apache.hadoop.ozone.om.OmSnapshot; import org.apache.hadoop.ozone.om.OmSnapshotManager; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; +import org.apache.hadoop.ozone.om.lock.OMLockDetails; import org.apache.hadoop.ozone.om.request.key.OMDirectoriesPurgeRequestWithFSO; import org.apache.hadoop.ozone.om.response.CleanupTableInfo; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; @@ -86,9 +91,15 @@ public void addToDBBatch(OMMetadataManager metadataManager, OmSnapshotManager omSnapshotManager = ((OmMetadataManagerImpl) metadataManager) .getOzoneManager().getOmSnapshotManager(); - + IOzoneManagerLock lock = metadataManager.getLock(); + UUID fromSnapshotId = fromSnapshotInfo.getSnapshotId(); + OMLockDetails lockDetails = lock.acquireReadLock(SNAPSHOT_DB_CONTENT_LOCK, fromSnapshotId.toString()); + if (!lockDetails.isLockAcquired()) { + throw new OMException("Unable to acquire read lock on " + SNAPSHOT_DB_CONTENT_LOCK + " for snapshot: " + + fromSnapshotId, OMException.ResultCodes.INTERNAL_ERROR); + } try (UncheckedAutoCloseableSupplier - rcFromSnapshotInfo = omSnapshotManager.getSnapshot(fromSnapshotInfo.getSnapshotId())) { + rcFromSnapshotInfo = omSnapshotManager.getSnapshot(fromSnapshotId)) { OmSnapshot fromSnapshot = rcFromSnapshotInfo.get(); DBStore fromSnapshotStore = fromSnapshot.getMetadataManager() .getStore(); @@ -98,6 +109,8 @@ public void addToDBBatch(OMMetadataManager metadataManager, processPaths(metadataManager, fromSnapshot.getMetadataManager(), batchOp, writeBatch); fromSnapshotStore.commitBatchOperation(writeBatch); } + } finally { + lock.releaseReadLock(SNAPSHOT_DB_CONTENT_LOCK, fromSnapshotId.toString()); } metadataManager.getSnapshotInfoTable().putWithBatch(batchOp, fromSnapshotInfo.getTableKey(), fromSnapshotInfo); } else { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyPurgeResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyPurgeResponse.java index 38ce0a6266c2..b9ba768f6cb6 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyPurgeResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyPurgeResponse.java @@ -19,21 +19,26 @@ import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DELETED_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.SNAPSHOT_INFO_TABLE; +import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_CONTENT_LOCK; import static org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotMoveDeletedKeysResponse.createRepeatedOmKeyInfo; import jakarta.annotation.Nonnull; import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.UUID; import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.hdds.utils.db.DBStore; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; import org.apache.hadoop.ozone.om.OmSnapshot; import org.apache.hadoop.ozone.om.OmSnapshotManager; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; +import org.apache.hadoop.ozone.om.lock.OMLockDetails; import org.apache.hadoop.ozone.om.request.key.OMKeyPurgeRequest; import org.apache.hadoop.ozone.om.response.CleanupTableInfo; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyInfo; @@ -82,10 +87,15 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, if (fromSnapshot != null) { OmSnapshotManager omSnapshotManager = ((OmMetadataManagerImpl) omMetadataManager).getOzoneManager().getOmSnapshotManager(); - + IOzoneManagerLock lock = omMetadataManager.getLock(); + UUID fromSnapshotId = fromSnapshot.getSnapshotId(); + OMLockDetails lockDetails = lock.acquireReadLock(SNAPSHOT_DB_CONTENT_LOCK, fromSnapshotId.toString()); + if (!lockDetails.isLockAcquired()) { + throw new OMException("Unable to acquire read lock on " + SNAPSHOT_DB_CONTENT_LOCK + " for snapshot: " + + fromSnapshotId, OMException.ResultCodes.INTERNAL_ERROR); + } try (UncheckedAutoCloseableSupplier rcOmFromSnapshot = - omSnapshotManager.getSnapshot(fromSnapshot.getSnapshotId())) { - + omSnapshotManager.getSnapshot(fromSnapshotId)) { OmSnapshot fromOmSnapshot = rcOmFromSnapshot.get(); DBStore fromSnapshotStore = fromOmSnapshot.getMetadataManager().getStore(); // Init Batch Operation for snapshot db. @@ -95,6 +105,8 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, processKeysToUpdate(writeBatch, fromOmSnapshot.getMetadataManager()); fromSnapshotStore.commitBatchOperation(writeBatch); } + } finally { + lock.releaseReadLock(SNAPSHOT_DB_CONTENT_LOCK, fromSnapshotId.toString()); } omMetadataManager.getSnapshotInfoTable().putWithBatch(batchOperation, fromSnapshot.getTableKey(), fromSnapshot); } else { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveTableKeysResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveTableKeysResponse.java index 3c40bafd0b06..1d85ca0f22a2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveTableKeysResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotMoveTableKeysResponse.java @@ -18,8 +18,10 @@ package org.apache.hadoop.ozone.om.response.snapshot; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.SNAPSHOT_INFO_TABLE; +import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_CONTENT_LOCK; import static org.apache.hadoop.ozone.om.snapshot.SnapshotUtils.createMergedRepeatedOmKeyInfoFromDeletedTableEntry; +import com.google.common.collect.Lists; import jakarta.annotation.Nonnull; import java.io.IOException; import java.util.List; @@ -30,9 +32,12 @@ import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; import org.apache.hadoop.ozone.om.OmSnapshot; import org.apache.hadoop.ozone.om.OmSnapshotManager; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; +import org.apache.hadoop.ozone.om.lock.OMLockDetails; import org.apache.hadoop.ozone.om.response.CleanupTableInfo; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -80,7 +85,15 @@ public OMSnapshotMoveTableKeysResponse(@Nonnull OMResponse omResponse) { protected void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { OmSnapshotManager omSnapshotManager = ((OmMetadataManagerImpl) omMetadataManager) .getOzoneManager().getOmSnapshotManager(); - + IOzoneManagerLock lock = omMetadataManager.getLock(); + String[] fromSnapshotId = new String[] {fromSnapshot.getSnapshotId().toString()}; + String[] nextSnapshotId = nextSnapshot == null ? null : new String[] {nextSnapshot.getSnapshotId().toString()}; + List snapshotIds = Lists.newArrayList(fromSnapshotId, nextSnapshotId); + OMLockDetails lockDetails = lock.acquireReadLocks(SNAPSHOT_DB_CONTENT_LOCK, snapshotIds); + if (!lockDetails.isLockAcquired()) { + throw new OMException("Unable to acquire read lock on " + SNAPSHOT_DB_CONTENT_LOCK + " for snapshot: " + + snapshotIds, OMException.ResultCodes.INTERNAL_ERROR); + } try (UncheckedAutoCloseableSupplier rcOmFromSnapshot = omSnapshotManager.getSnapshot(fromSnapshot.getSnapshotId())) { @@ -113,6 +126,8 @@ protected void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation fromSnapshotStore.getDb().flushWal(true); fromSnapshotStore.getDb().flush(); } + } finally { + lock.releaseReadLocks(SNAPSHOT_DB_CONTENT_LOCK, snapshotIds); } // Flush snapshot info to rocksDB. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java index db44337ee411..ab40a0530fce 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java @@ -117,7 +117,7 @@ public SnapshotDeletingService(long interval, long serviceTimeout, OZONE_SNAPSHOT_KEY_DELETING_LIMIT_PER_TASK, OZONE_SNAPSHOT_KEY_DELETING_LIMIT_PER_TASK_DEFAULT); IOzoneManagerLock lock = getOzoneManager().getMetadataManager().getLock(); - this.snapshotIdLocks = new MultiSnapshotLocks(lock, SNAPSHOT_GC_LOCK, true); + this.snapshotIdLocks = new MultiSnapshotLocks(lock, SNAPSHOT_GC_LOCK, true, 2); this.lockIds = new ArrayList<>(2); } 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 525877306965..bb8161f0faeb 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 @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.om.snapshot; +import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -39,11 +40,16 @@ public class MultiSnapshotLocks { private final boolean writeLock; private OMLockDetails lockDetails; + @VisibleForTesting public MultiSnapshotLocks(IOzoneManagerLock lock, Resource resource, boolean writeLock) { + this(lock, resource, writeLock, 0); + } + + public MultiSnapshotLocks(IOzoneManagerLock lock, Resource resource, boolean writeLock, int maxNumberOfLocks) { this.writeLock = writeLock; this.resource = resource; this.lock = lock; - this.objectLocks = new ArrayList<>(); + this.objectLocks = new ArrayList<>(maxNumberOfLocks); this.lockDetails = OMLockDetails.EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } 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 7d227dfb641c..89c0e4c46e20 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 @@ -90,7 +90,8 @@ public ReclaimableFilter( this.omSnapshotManager = omSnapshotManager; this.currentSnapshotInfo = currentSnapshotInfo; this.snapshotChainManager = snapshotChainManager; - this.snapshotIdLocks = new MultiSnapshotLocks(lock, SNAPSHOT_GC_LOCK, false); + this.snapshotIdLocks = new MultiSnapshotLocks(lock, SNAPSHOT_GC_LOCK, false, + numberOfPreviousSnapshotsFromChain + 1); this.keyManager = keyManager; this.numberOfPreviousSnapshotsFromChain = numberOfPreviousSnapshotsFromChain; this.previousOmSnapshots = new ArrayList<>(numberOfPreviousSnapshotsFromChain); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMDirectoriesPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMDirectoriesPurgeRequestAndResponse.java index 54087fa64dc1..881a4dff939d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMDirectoriesPurgeRequestAndResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMDirectoriesPurgeRequestAndResponse.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.om.request.key; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE; +import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_CONTENT_LOCK; import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.LeveledResource.BUCKET_LOCK; import static org.apache.hadoop.ozone.om.request.file.OMFileRequest.getOmKeyInfo; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -32,6 +33,7 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import com.google.common.collect.Lists; import jakarta.annotation.Nonnull; import java.io.IOException; import java.util.ArrayList; @@ -433,7 +435,24 @@ public void testDirectoryPurge(boolean fromSnapshot, boolean purgeDirectory, int OMDirectoriesPurgeRequestWithFSO omKeyPurgeRequest = new OMDirectoriesPurgeRequestWithFSO(preExecutedRequest); OMDirectoriesPurgeResponseWithFSO omClientResponse = (OMDirectoriesPurgeResponseWithFSO) omKeyPurgeRequest .validateAndUpdateCache(ozoneManager, 100L); + + IOzoneManagerLock lock = spy(omMetadataManager.getLock()); + when(omMetadataManager.getLock()).thenReturn(lock); + List locks = Lists.newArrayList(); + doAnswer(i -> { + locks.add(i.getArgument(1)); + return i.callRealMethod(); + }).when(lock).acquireReadLock(eq(SNAPSHOT_DB_CONTENT_LOCK), anyString()); + + List snapshotIds; + if (fromSnapshot) { + snapshotIds = Collections.singletonList(snapshotInfo.getSnapshotId().toString()); + } else { + snapshotIds = Collections.emptyList(); + } + performBatchOperationCommit(omClientResponse); + assertEquals(snapshotIds, locks); OmBucketInfo updatedBucketInfo = purgeDirectory || numberOfSubEntries > 0 ? omMetadataManager.getBucketTable().getSkipCache(bucketKey) : omMetadataManager.getBucketTable().get(bucketKey); long currentSnapshotUsedNamespace = updatedBucketInfo.getSnapshotUsedNamespace(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java index aa566859cb46..a7a738ba0000 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java @@ -17,14 +17,21 @@ package org.apache.hadoop.ozone.om.request.key; +import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_CONTENT_LOCK; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import com.google.common.collect.Lists; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.UUID; import org.apache.commons.lang3.tuple.Pair; @@ -35,6 +42,7 @@ import org.apache.hadoop.ozone.om.OmSnapshot; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.om.response.key.OMKeyPurgeResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeletedKeys; @@ -186,7 +194,6 @@ public void testKeyPurgeInSnapshot() throws Exception { .thenReturn(RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.THREE)); // Create and Delete keys. The keys should be moved to DeletedKeys table Pair, List> deleteKeysAndRenamedEntry = createAndDeleteKeysAndRenamedEntry(1, null); - SnapshotInfo snapInfo = createSnapshot("snap1"); assertEquals(snapInfo.getLastTransactionInfo(), TransactionInfo.valueOf(TransactionInfo.getTermIndex(1L)).toByteString()); @@ -235,6 +242,14 @@ public void testKeyPurgeInSnapshot() throws Exception { .setStatus(Status.OK) .build(); + IOzoneManagerLock lock = spy(omMetadataManager.getLock()); + when(omMetadataManager.getLock()).thenReturn(lock); + List locks = Lists.newArrayList(); + doAnswer(i -> { + locks.add(i.getArgument(1)); + return i.callRealMethod(); + }).when(lock).acquireReadLock(eq(SNAPSHOT_DB_CONTENT_LOCK), anyString()); + List snapshotIds = Collections.singletonList(snapInfo.getSnapshotId().toString()); try (BatchOperation batchOperation = omMetadataManager.getStore().initBatchOperation()) { @@ -245,6 +260,7 @@ public void testKeyPurgeInSnapshot() throws Exception { // Do manual commit and see whether addToBatch is successful or not. omMetadataManager.getStore().commitBatchOperation(batchOperation); } + assertEquals(snapshotIds, locks); snapshotInfoOnDisk = omMetadataManager.getSnapshotInfoTable().getSkipCache(snapInfo.getTableKey()); assertEquals(snapshotInfoOnDisk, snapInfo); // The keys should not exist in the DeletedKeys table diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java index f0e32ac405ba..b84294370c58 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java @@ -349,5 +349,4 @@ protected SnapshotInfo createSnapshot(String volume, String bucket, String snaps assertNotNull(snapshotInfo); return snapshotInfo; } - } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotMoveTableKeysResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotMoveTableKeysResponse.java index 0425dd84546f..db72781f753c 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotMoveTableKeysResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotMoveTableKeysResponse.java @@ -17,8 +17,19 @@ package org.apache.hadoop.ozone.om.response.snapshot; +import static org.apache.hadoop.ozone.om.lock.FlatResource.SNAPSHOT_DB_CONTENT_LOCK; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -37,6 +48,7 @@ import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; import org.apache.hadoop.ozone.om.request.key.OMKeyRequest; import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils; import org.apache.hadoop.ozone.om.snapshot.TestSnapshotRequestAndResponse; @@ -107,13 +119,24 @@ private void addDataToTable(Table table, List> va @ParameterizedTest @ValueSource(booleans = {true, false}) public void testMoveTableKeysToNextSnapshot(boolean nextSnapshotExists) throws Exception { + IOzoneManagerLock lock = spy(getOmMetadataManager().getLock()); + when(getOmMetadataManager().getLock()).thenReturn(lock); OmBucketInfo omBucketInfo = OMKeyRequest.getBucketInfo(getOmMetadataManager(), getVolumeName(), getBucketName()); createSnapshots(nextSnapshotExists, omBucketInfo.getObjectID()); try (UncheckedAutoCloseableSupplier snapshot1 = getOmSnapshotManager().getSnapshot( getVolumeName(), getBucketName(), snapshotName1); UncheckedAutoCloseableSupplier snapshot2 = nextSnapshotExists ? getOmSnapshotManager().getSnapshot( getVolumeName(), getBucketName(), snapshotName2) : null) { - + List> expectedSnapshotIdLocks = + Arrays.asList(Collections.singletonList(snapshot1.get().getSnapshotID().toString()), + nextSnapshotExists ? Collections.singletonList(snapshot2.get().getSnapshotID().toString()) : null); + List> locks = new ArrayList<>(); + doAnswer(i -> { + for (String[] id : (Collection)i.getArgument(1)) { + locks.add(id == null ? null : Arrays.stream(id).collect(Collectors.toList())); + } + return i.callRealMethod(); + }).when(lock).acquireReadLocks(eq(SNAPSHOT_DB_CONTENT_LOCK), anyList()); OmSnapshot snapshot = snapshot1.get(); List deletedTable = new ArrayList<>(); List deletedDirTable = new ArrayList<>(); @@ -144,6 +167,7 @@ public void testMoveTableKeysToNextSnapshot(boolean nextSnapshotExists) throws E response.addToDBBatch(getOmMetadataManager(), batchOperation); getOmMetadataManager().getStore().commitBatchOperation(batchOperation); } + assertEquals(expectedSnapshotIdLocks, locks); Assertions.assertTrue(snapshot.getMetadataManager().getDeletedTable().isEmpty()); Assertions.assertTrue(snapshot.getMetadataManager().getDeletedDirTable().isEmpty()); Assertions.assertTrue(snapshot.getMetadataManager().getSnapshotRenamedTable().isEmpty()); @@ -153,7 +177,7 @@ public void testMoveTableKeysToNextSnapshot(boolean nextSnapshotExists) throws E nextMetadataManager.getDeletedTable().iterator().forEachRemaining(entry -> { count.getAndIncrement(); int maxCount = count.get() >= 6 && count.get() <= 8 ? 20 : 10; - Assertions.assertEquals(maxCount, entry.getValue().getOmKeyInfoList().size()); + assertEquals(maxCount, entry.getValue().getOmKeyInfoList().size()); List versions = entry.getValue().getOmKeyInfoList().stream().map(OmKeyInfo::getKeyLocationVersions) .map(omKeyInfo -> omKeyInfo.get(0).getVersion()).collect(Collectors.toList()); List expectedVersions = new ArrayList<>(); @@ -161,20 +185,20 @@ public void testMoveTableKeysToNextSnapshot(boolean nextSnapshotExists) throws E expectedVersions.addAll(LongStream.range(10, 20).boxed().collect(Collectors.toList())); } expectedVersions.addAll(LongStream.range(0, 10).boxed().collect(Collectors.toList())); - Assertions.assertEquals(expectedVersions, versions); + assertEquals(expectedVersions, versions); }); - Assertions.assertEquals(15, count.get()); + assertEquals(15, count.get()); count.set(0); nextMetadataManager.getDeletedDirTable().iterator().forEachRemaining(entry -> count.getAndIncrement()); - Assertions.assertEquals(15, count.get()); + assertEquals(15, count.get()); count.set(0); nextMetadataManager.getSnapshotRenamedTable().iterator().forEachRemaining(entry -> { String expectedValue = renameEntries.getOrDefault(entry.getKey(), entry.getValue()); - Assertions.assertEquals(expectedValue, entry.getValue()); + assertEquals(expectedValue, entry.getValue()); count.getAndIncrement(); }); - Assertions.assertEquals(15, count.get()); + assertEquals(15, count.get()); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotRequestAndResponse.java index d9e81693dd8d..2e0abc07da25 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotRequestAndResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotRequestAndResponse.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.framework; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import java.io.File; @@ -148,8 +149,8 @@ public void baseSetup() throws Exception { testDir.getAbsolutePath()); ozoneConfiguration.set(OzoneConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath()); - omMetadataManager = new OmMetadataManagerImpl(ozoneConfiguration, - ozoneManager); + omMetadataManager = spy(new OmMetadataManagerImpl(ozoneConfiguration, + ozoneManager)); when(ozoneManager.getConfiguration()).thenReturn(ozoneConfiguration); when(ozoneManager.resolveBucketLink(any(Pair.class), any(OMClientRequest.class))) .thenAnswer(i -> new ResolvedBucket(i.getArgument(0), @@ -276,5 +277,4 @@ protected List>> getDeletedDirKeys(String volume, S }) .collect(Collectors.toList()); } - }