diff --git a/hadoop-hdds/hadoop-dependency-test/pom.xml b/hadoop-hdds/hadoop-dependency-test/pom.xml index c226c184720a..308c9dad5abc 100644 --- a/hadoop-hdds/hadoop-dependency-test/pom.xml +++ b/hadoop-hdds/hadoop-dependency-test/pom.xml @@ -67,5 +67,15 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> assertj-core compile + + + org.junit.jupiter + junit-jupiter-engine + + + org.junit.vintage + junit-vintage-engine + test + diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientWithRatis.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientWithRatis.java index 3a8d117bc6d5..8b2367c6b35f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientWithRatis.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientWithRatis.java @@ -60,6 +60,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartInfo; import org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine; import org.apache.hadoop.ozone.om.ratis.metrics.OzoneManagerStateMachineMetrics; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; import org.apache.ozone.test.GenericTestUtils; import org.junit.Assert; import org.junit.jupiter.api.AfterAll; @@ -340,9 +341,11 @@ public void testParallelDeleteBucketAndCreateKey() throws IOException, omSM.getHandler().setInjector(injector); thread1.start(); thread2.start(); - GenericTestUtils.waitFor(() -> metrics.getApplyTransactionMapSize() > 0, - 100, 5000); - Thread.sleep(2000); + // Wait long enough for createKey's preExecute to finish executing + GenericTestUtils.waitFor(() -> { + return getCluster().getOzoneManager().getOmServerProtocol().getLastRequestToSubmit().getCmdType().equals( + Type.CreateKey); + }, 100, 10000); injector.resume(); try { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java index cd932f6efde8..526ff8e2013e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hdds.utils.TransactionInfo; import org.apache.hadoop.hdds.utils.db.DBCheckpoint; import org.apache.hadoop.hdds.utils.db.RDBCheckpointUtils; +import org.apache.hadoop.hdds.utils.db.RDBStore; import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.MiniOzoneHAClusterImpl; import org.apache.hadoop.ozone.client.BucketArgs; @@ -60,8 +61,8 @@ import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.io.TempDir; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; +import org.rocksdb.RocksDB; +import org.rocksdb.RocksDBException; import org.slf4j.Logger; import org.slf4j.event.Level; @@ -198,11 +199,12 @@ public void shutdown() { } } - @ParameterizedTest - @ValueSource(ints = {100}) // tried up to 1000 snapshots and this test works, but some of the // timeouts have to be increased. - public void testInstallSnapshot(int numSnapshotsToCreate) throws Exception { + private static final int SNAPSHOTS_TO_CREATE = 100; + + @Test + public void testInstallSnapshot(@TempDir Path tempDir) throws Exception { // Get the leader OM String leaderOMNodeId = OmFailoverProxyUtil .getFailoverProxyProvider(objectStore.getClientProxy()) @@ -230,8 +232,7 @@ public void testInstallSnapshot(int numSnapshotsToCreate) throws Exception { String snapshotName = ""; List keys = new ArrayList<>(); SnapshotInfo snapshotInfo = null; - for (int snapshotCount = 0; snapshotCount < numSnapshotsToCreate; - snapshotCount++) { + for (int snapshotCount = 0; snapshotCount < SNAPSHOTS_TO_CREATE; snapshotCount++) { snapshotName = snapshotNamePrefix + snapshotCount; keys = writeKeys(keyIncrement); snapshotInfo = createOzoneSnapshot(leaderOM, snapshotName); @@ -326,7 +327,7 @@ public void testInstallSnapshot(int numSnapshotsToCreate) throws Exception { private void checkSnapshot(OzoneManager leaderOM, OzoneManager followerOM, String snapshotName, List keys, SnapshotInfo snapshotInfo) - throws IOException { + throws IOException, RocksDBException { // Read back data from snapshot. OmKeyArgs omKeyArgs = new OmKeyArgs.Builder() .setVolumeName(volumeName) @@ -347,10 +348,19 @@ private void checkSnapshot(OzoneManager leaderOM, OzoneManager followerOM, Path leaderActiveDir = Paths.get(leaderMetaDir.toString(), OM_DB_NAME); Path leaderSnapshotDir = Paths.get(getSnapshotPath(leaderOM.getConfiguration(), snapshotInfo)); + + // Get list of live files on the leader. + RocksDB activeRocksDB = ((RDBStore) leaderOM.getMetadataManager().getStore()) + .getDb().getManagedRocksDb().get(); + // strip the leading "/". + Set liveSstFiles = activeRocksDB.getLiveFiles().files.stream() + .map(s -> s.substring(1)) + .collect(Collectors.toSet()); + // Get the list of hardlinks from the leader. Then confirm those links // are on the follower int hardLinkCount = 0; - try (Streamlist = Files.list(leaderSnapshotDir)) { + try (Stream list = Files.list(leaderSnapshotDir)) { for (Path leaderSnapshotSST: list.collect(Collectors.toList())) { String fileName = leaderSnapshotSST.getFileName().toString(); if (fileName.toLowerCase().endsWith(".sst")) { @@ -358,7 +368,8 @@ private void checkSnapshot(OzoneManager leaderOM, OzoneManager followerOM, Path leaderActiveSST = Paths.get(leaderActiveDir.toString(), fileName); // Skip if not hard link on the leader - if (!leaderActiveSST.toFile().exists()) { + // First confirm it is live + if (!liveSstFiles.contains(fileName)) { continue; } // If it is a hard link on the leader, it should be a hard diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotBackgroundServices.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotBackgroundServices.java index 8bd78460c1bd..528dcec4def1 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotBackgroundServices.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotBackgroundServices.java @@ -572,7 +572,7 @@ private void checkIfSnapshotGetsProcessedBySFS(OzoneManager ozoneManager) } catch (IOException e) { Assertions.fail(); } - return snapshotInfo.isSstFiltered(); + return SstFilteringService.isSstFiltered(ozoneManager.getConfiguration(), snapshotInfo); }, 1000, 10000); } diff --git a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java index 68c5cf758eb5..1ff185fa3555 100644 --- a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java +++ b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java @@ -46,8 +46,7 @@ import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; import org.apache.hadoop.hdds.utils.TransactionInfo; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.ExpiredMultipartUploadsBucket; -import org.apache.hadoop.ozone.storage.proto. - OzoneManagerStorageProtos.PersistedUserVolumeInfo; +import org.apache.hadoop.ozone.storage.proto.OzoneManagerStorageProtos.PersistedUserVolumeInfo; import org.apache.hadoop.ozone.security.OzoneTokenIdentifier; import org.apache.hadoop.hdds.utils.db.DBStore; import org.apache.hadoop.hdds.utils.db.Table; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java index 806b8b39e901..02a9e8babe1e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java @@ -731,6 +731,13 @@ public static String getSnapshotPrefix(String snapshotName) { snapshotName + OM_KEY_PREFIX; } + public static Path getSnapshotPath(OMMetadataManager omMetadataManager, SnapshotInfo snapshotInfo) { + RDBStore store = (RDBStore) omMetadataManager.getStore(); + String checkpointPrefix = store.getDbLocation().getName(); + return Paths.get(store.getSnapshotsParentDir(), + checkpointPrefix + snapshotInfo.getCheckpointDir()); + } + public static String getSnapshotPath(OzoneConfiguration conf, SnapshotInfo snapshotInfo) { return OMStorage.getOmDbDir(conf) + diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java index 20d0ab0e53eb..52e5da504038 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java @@ -19,6 +19,7 @@ package org.apache.hadoop.ozone.om; import com.google.common.annotations.VisibleForTesting; +import org.apache.hadoop.hdds.StringUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.utils.BackgroundService; import org.apache.hadoop.hdds.utils.BackgroundTask; @@ -38,6 +39,9 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -69,6 +73,10 @@ public class SstFilteringService extends BackgroundService // multiple times. private static final int SST_FILTERING_CORE_POOL_SIZE = 1; + public static final String SST_FILTERED_FILE = "sstFiltered"; + private static final byte[] SST_FILTERED_FILE_CONTENT = StringUtils.string2Bytes("This file holds information " + + "if a particular snapshot has filtered out the relevant sst files or not.\nDO NOT add, change or delete " + + "any files in this directory unless you know what you are doing.\n"); private final OzoneManager ozoneManager; // Number of files to be batched in an iteration. @@ -78,6 +86,12 @@ public class SstFilteringService extends BackgroundService private AtomicBoolean running; + public static boolean isSstFiltered(OzoneConfiguration ozoneConfiguration, SnapshotInfo snapshotInfo) { + Path sstFilteredFile = Paths.get(OmSnapshotManager.getSnapshotPath(ozoneConfiguration, + snapshotInfo), SST_FILTERED_FILE); + return snapshotInfo.isSstFiltered() || sstFilteredFile.toFile().exists(); + } + public SstFilteringService(long interval, TimeUnit unit, long serviceTimeout, OzoneManager ozoneManager, OzoneConfiguration configuration) { super("SstFilteringService", interval, unit, SST_FILTERING_CORE_POOL_SIZE, @@ -112,33 +126,35 @@ public void resume() { private class SstFilteringTask implements BackgroundTask { + private boolean isSnapshotDeleted(SnapshotInfo snapshotInfo) { + return snapshotInfo == null || snapshotInfo.getSnapshotStatus() == SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED; + } + /** - * Marks the SSTFiltered flag corresponding to the snapshot. - * @param volume Volume name of the snapshot - * @param bucket Bucket name of the snapshot - * @param snapshotName Snapshot name + * Marks the snapshot as SSTFiltered by creating a file in snapshot directory. + * @param snapshotInfo snapshotInfo * @throws IOException */ - private void markSSTFilteredFlagForSnapshot(String volume, String bucket, - String snapshotName) throws IOException { + private void markSSTFilteredFlagForSnapshot(SnapshotInfo snapshotInfo) throws IOException { + // Acquiring read lock to avoid race condition with the snapshot directory deletion occurring + // in OmSnapshotPurgeResponse. Any operation apart from delete can run in parallel along with this operation. + //TODO. Revisit other SNAPSHOT_LOCK and see if we can change write locks to read locks to further optimize it. OMLockDetails omLockDetails = ozoneManager.getMetadataManager().getLock() - .acquireWriteLock(SNAPSHOT_LOCK, volume, bucket, snapshotName); + .acquireReadLock(SNAPSHOT_LOCK, snapshotInfo.getVolumeName(), snapshotInfo.getBucketName(), + snapshotInfo.getName()); boolean acquiredSnapshotLock = omLockDetails.isLockAcquired(); if (acquiredSnapshotLock) { - Table snapshotInfoTable = - ozoneManager.getMetadataManager().getSnapshotInfoTable(); + String snapshotDir = OmSnapshotManager.getSnapshotPath(ozoneManager.getConfiguration(), snapshotInfo); try { - // mark the snapshot as filtered by writing to the file - String snapshotTableKey = SnapshotInfo.getTableKey(volume, bucket, - snapshotName); - SnapshotInfo snapshotInfo = snapshotInfoTable.get(snapshotTableKey); - - snapshotInfo.setSstFiltered(true); - snapshotInfoTable.put(snapshotTableKey, snapshotInfo); + // mark the snapshot as filtered by creating a file. + if (Files.exists(Paths.get(snapshotDir))) { + Files.write(Paths.get(snapshotDir, SST_FILTERED_FILE), SST_FILTERED_FILE_CONTENT); + } } finally { ozoneManager.getMetadataManager().getLock() - .releaseWriteLock(SNAPSHOT_LOCK, volume, bucket, snapshotName); + .releaseReadLock(SNAPSHOT_LOCK, snapshotInfo.getVolumeName(), + snapshotInfo.getBucketName(), snapshotInfo.getName()); } } } @@ -163,12 +179,11 @@ public BackgroundTaskResult call() throws Exception { long snapshotLimit = snapshotLimitPerTask; while (iterator.hasNext() && snapshotLimit > 0 && running.get()) { + Table.KeyValue keyValue = iterator.next(); + String snapShotTableKey = keyValue.getKey(); + SnapshotInfo snapshotInfo = keyValue.getValue(); try { - Table.KeyValue keyValue = iterator.next(); - String snapShotTableKey = keyValue.getKey(); - SnapshotInfo snapshotInfo = keyValue.getValue(); - - if (snapshotInfo.isSstFiltered()) { + if (isSstFiltered(ozoneManager.getConfiguration(), snapshotInfo)) { continue; } @@ -194,6 +209,9 @@ public BackgroundTaskResult call() throws Exception { .lock()) { db.deleteFilesNotMatchingPrefix(columnFamilyNameToPrefixMap); } + markSSTFilteredFlagForSnapshot(snapshotInfo); + snapshotLimit--; + snapshotFilteredCount.getAndIncrement(); } catch (OMException ome) { // FILE_NOT_FOUND is obtained when the snapshot is deleted // In this case, get the snapshotInfo from the db, check if @@ -202,20 +220,22 @@ public BackgroundTaskResult call() throws Exception { SnapshotInfo snapshotInfoToCheck = ozoneManager.getMetadataManager().getSnapshotInfoTable() .get(snapShotTableKey); - if (snapshotInfoToCheck.getSnapshotStatus() == - SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED) { + if (isSnapshotDeleted(snapshotInfoToCheck)) { LOG.info("Snapshot with name: '{}', id: '{}' has been " + "deleted.", snapshotInfo.getName(), snapshotInfo .getSnapshotId()); } } } - markSSTFilteredFlagForSnapshot(snapshotInfo.getVolumeName(), - snapshotInfo.getBucketName(), snapshotInfo.getName()); - snapshotLimit--; - snapshotFilteredCount.getAndIncrement(); } catch (RocksDBException | IOException e) { - LOG.error("Exception encountered while filtering a snapshot", e); + if (isSnapshotDeleted(snapshotInfoTable.get(snapShotTableKey))) { + LOG.info("Exception encountered while filtering a snapshot: {} since it was deleted midway", + snapShotTableKey, e); + } else { + LOG.error("Exception encountered while filtering a snapshot", e); + } + + } } } catch (IOException e) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java index 367f1febc732..c634198df0a3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java @@ -20,10 +20,11 @@ import org.apache.commons.io.FileUtils; import org.apache.hadoop.hdds.utils.db.BatchOperation; -import org.apache.hadoop.hdds.utils.db.RDBStore; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; +import org.apache.hadoop.ozone.om.OmSnapshotManager; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +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; @@ -33,11 +34,11 @@ import javax.annotation.Nonnull; import java.io.IOException; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.List; import java.util.Map; import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.SNAPSHOT_INFO_TABLE; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.SNAPSHOT_LOCK; /** * Response for OMSnapshotPurgeRequest. @@ -117,15 +118,24 @@ private void updateSnapInfo(OmMetadataManagerImpl metadataManager, */ private void deleteCheckpointDirectory(OMMetadataManager omMetadataManager, SnapshotInfo snapshotInfo) { - RDBStore store = (RDBStore) omMetadataManager.getStore(); - String checkpointPrefix = store.getDbLocation().getName(); - Path snapshotDirPath = Paths.get(store.getSnapshotsParentDir(), - checkpointPrefix + snapshotInfo.getCheckpointDir()); - try { - FileUtils.deleteDirectory(snapshotDirPath.toFile()); - } catch (IOException ex) { - LOG.error("Failed to delete snapshot directory {} for snapshot {}", - snapshotDirPath, snapshotInfo.getTableKey(), ex); + // Acquiring write lock to avoid race condition with sst filtering service which creates a sst filtered file + // inside the snapshot directory. Any operation apart which doesn't create/delete files under this snapshot + // directory can run in parallel along with this operation. + OMLockDetails omLockDetails = omMetadataManager.getLock() + .acquireWriteLock(SNAPSHOT_LOCK, snapshotInfo.getVolumeName(), snapshotInfo.getBucketName(), + snapshotInfo.getName()); + boolean acquiredSnapshotLock = omLockDetails.isLockAcquired(); + if (acquiredSnapshotLock) { + Path snapshotDirPath = OmSnapshotManager.getSnapshotPath(omMetadataManager, snapshotInfo); + try { + FileUtils.deleteDirectory(snapshotDirPath.toFile()); + } catch (IOException ex) { + LOG.error("Failed to delete snapshot directory {} for snapshot {}", + snapshotDirPath, snapshotInfo.getTableKey(), ex); + } finally { + omMetadataManager.getLock().releaseWriteLock(SNAPSHOT_LOCK, snapshotInfo.getVolumeName(), + snapshotInfo.getBucketName(), snapshotInfo.getName()); + } } } } 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 b1f4b1106ebc..a98081c63a17 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 @@ -36,7 +36,6 @@ import org.apache.hadoop.ozone.lock.BootstrapStateHandler; import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; -import org.apache.hadoop.ozone.om.KeyManagerImpl; import org.apache.hadoop.ozone.om.OmSnapshot; import org.apache.hadoop.ozone.om.OmSnapshotManager; import org.apache.hadoop.ozone.om.OzoneManager; @@ -153,12 +152,9 @@ public BackgroundTaskResult call() throws InterruptedException { while (iterator.hasNext() && snapshotLimit > 0) { SnapshotInfo snapInfo = iterator.next().getValue(); - boolean isSstFilteringServiceEnabled = - ((KeyManagerImpl) ozoneManager.getKeyManager()) - .isSstFilteringSvcEnabled(); // Only Iterate in deleted snapshot - if (shouldIgnoreSnapshot(snapInfo, isSstFilteringServiceEnabled)) { + if (shouldIgnoreSnapshot(snapInfo)) { continue; } @@ -590,11 +586,10 @@ public void submitRequest(OMRequest omRequest) { } } - public static boolean shouldIgnoreSnapshot(SnapshotInfo snapInfo, - boolean isSstFilteringServiceEnabled) { + @VisibleForTesting + boolean shouldIgnoreSnapshot(SnapshotInfo snapInfo) { SnapshotInfo.SnapshotStatus snapshotStatus = snapInfo.getSnapshotStatus(); - return !(snapshotStatus == SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED) - || (isSstFilteringServiceEnabled && !snapInfo.isSstFiltered()); + return snapshotStatus != SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED; } // TODO: Move this util class. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 11b6cc26c815..fe98a73ea6cb 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -88,6 +88,9 @@ public class OzoneManagerProtocolServerSideTranslatorPB implements // always true, only used in tests private boolean shouldFlushCache = true; + private OMRequest lastRequestToSubmit; + + /** * Constructs an instance of the server handler. * @@ -225,6 +228,7 @@ private OMResponse internalProcessRequest(OMRequest request) throws assert (omClientRequest != null); OMClientRequest finalOmClientRequest = omClientRequest; requestToSubmit = preExecute(finalOmClientRequest); + this.lastRequestToSubmit = requestToSubmit; } catch (IOException ex) { if (omClientRequest != null) { omClientRequest.handleRequestFailure(ozoneManager); @@ -248,6 +252,11 @@ private OMRequest preExecute(OMClientRequest finalOmClientRequest) () -> finalOmClientRequest.preExecute(ozoneManager)); } + @VisibleForTesting + public OMRequest getLastRequestToSubmit() { + return lastRequestToSubmit; + } + /** * Submits request to OM's Ratis server. */ diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestSstFilteringService.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestSstFilteringService.java index 678efabc3180..ae32268e5e2f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestSstFilteringService.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestSstFilteringService.java @@ -35,19 +35,22 @@ import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.om.snapshot.ReferenceCounted; -import org.apache.hadoop.security.authentication.client.AuthenticationException; import org.apache.ratis.util.ExitUtils; import org.awaitility.core.ConditionTimeoutException; -import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.MethodOrderer.OrderAnnotation; import org.junit.jupiter.api.io.TempDir; import org.rocksdb.LiveFileMetaData; import java.io.File; import java.io.IOException; import java.time.Duration; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -63,6 +66,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL; import static org.awaitility.Awaitility.with; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -71,24 +75,21 @@ /** * Test SST Filtering Service. */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@TestMethodOrder(OrderAnnotation.class) public class TestSstFilteringService { - public static final String SST_FILE_EXTENSION = ".sst"; - @TempDir - private File folder; + private static final String SST_FILE_EXTENSION = ".sst"; private OzoneManagerProtocol writeClient; private OzoneManager om; private OzoneConfiguration conf; private KeyManager keyManager; + private short countTotalSnapshots = 0; @BeforeAll - public static void setup() { + void setup(@TempDir Path folder) throws Exception { ExitUtils.disableSystemExit(); - } - - @BeforeEach - public void init() throws AuthenticationException, IOException { conf = new OzoneConfiguration(); - conf.set(OZONE_METADATA_DIRS, folder.getAbsolutePath()); + conf.set(OZONE_METADATA_DIRS, folder.toString()); conf.setTimeDuration(HDDS_CONTAINER_REPORT_INTERVAL, 200, TimeUnit.MILLISECONDS); conf.setTimeDuration(OZONE_SNAPSHOT_SST_FILTERING_SERVICE_INTERVAL, 100, @@ -102,7 +103,7 @@ public void init() throws AuthenticationException, IOException { om = omTestManagers.getOzoneManager(); } - @AfterEach + @AfterAll public void cleanup() throws Exception { if (keyManager != null) { keyManager.stop(); @@ -133,6 +134,7 @@ public void cleanup() throws Exception { * @throws IOException - on Failure. */ @Test + @Order(1) public void testIrrelevantSstFileDeletion() throws IOException, InterruptedException { RDBStore activeDbStore = (RDBStore) om.getMetadataManager().getStore(); @@ -142,7 +144,8 @@ public void testIrrelevantSstFileDeletion() final int keyCount = 100; String volumeName = "vol1"; String bucketName1 = "buck1"; - createVolumeAndBucket(volumeName, bucketName1); + createVolume(volumeName); + addBucketToVolume(volumeName, bucketName1); createKeys(volumeName, bucketName1, keyCount / 2); activeDbStore.getDb().flush(OmMetadataManagerImpl.KEY_TABLE); @@ -180,16 +183,16 @@ public void testIrrelevantSstFileDeletion() assertTrue(nonLevel0FilesCountAfterCompact > 0); String bucketName2 = "buck2"; - createVolumeAndBucket(volumeName, bucketName2); + addBucketToVolume(volumeName, bucketName2); createKeys(volumeName, bucketName2, keyCount); activeDbStore.getDb().flush(OmMetadataManagerImpl.KEY_TABLE); List allFiles = activeDbStore.getDb().getSstFileList(); String snapshotName1 = "snapshot1"; - writeClient.createSnapshot(volumeName, bucketName2, snapshotName1); + createSnapshot(volumeName, bucketName2, snapshotName1); SnapshotInfo snapshotInfo = om.getMetadataManager().getSnapshotInfoTable() .get(SnapshotInfo.getTableKey(volumeName, bucketName2, snapshotName1)); - assertFalse(snapshotInfo.isSstFiltered()); + assertFalse(SstFilteringService.isSstFiltered(om.getConfiguration(), snapshotInfo)); with().atMost(Duration.ofSeconds(120)) .pollInterval(Duration.ofSeconds(1)) .await() @@ -220,14 +223,18 @@ public void testIrrelevantSstFileDeletion() } } - assertTrue(snapshotInfo.isSstFiltered()); + // Need to read the sstFiltered flag which is set in background process and + // hence snapshotInfo.isSstFiltered() may not work sometimes. + assertTrue(SstFilteringService.isSstFiltered(om.getConfiguration(), + om.getMetadataManager().getSnapshotInfoTable().get(SnapshotInfo + .getTableKey(volumeName, bucketName2, snapshotName1)))); String snapshotName2 = "snapshot2"; long count; try (BootstrapStateHandler.Lock lock = filteringService.getBootstrapStateLock().lock()) { count = filteringService.getSnapshotFilteredCount().get(); - writeClient.createSnapshot(volumeName, bucketName2, snapshotName2); + createSnapshot(volumeName, bucketName2, snapshotName2); assertThrows(ConditionTimeoutException.class, () -> with() .atMost(Duration.ofSeconds(10)) @@ -252,14 +259,16 @@ public void testIrrelevantSstFileDeletion() } @Test + @Order(2) public void testActiveAndDeletedSnapshotCleanup() throws IOException { RDBStore activeDbStore = (RDBStore) om.getMetadataManager().getStore(); String volumeName = "volume1"; List bucketNames = Arrays.asList("bucket1", "bucket2"); + createVolume(volumeName); // Create 2 Buckets for (String bucketName : bucketNames) { - createVolumeAndBucket(volumeName, bucketName); + addBucketToVolume(volumeName, bucketName); } // Write 25 keys in each bucket, 2 sst files would be generated each for // keys in a single bucket @@ -277,8 +286,8 @@ public void testActiveAndDeletedSnapshotCleanup() throws IOException { keyManager.getSnapshotSstFilteringService(); sstFilteringService.pause(); - writeClient.createSnapshot(volumeName, bucketNames.get(0), "snap1"); - writeClient.createSnapshot(volumeName, bucketNames.get(0), "snap2"); + createSnapshot(volumeName, bucketNames.get(0), "snap1"); + createSnapshot(volumeName, bucketNames.get(0), "snap2"); SnapshotInfo snapshot1Info = om.getMetadataManager().getSnapshotInfoTable() .get(SnapshotInfo.getTableKey(volumeName, bucketNames.get(0), "snap1")); @@ -297,17 +306,17 @@ public void testActiveAndDeletedSnapshotCleanup() throws IOException { .await().until(() -> snap1Current.exists() && snap2Current.exists()); long snap1SstFileCountBeforeFilter = Arrays.stream(snapshot1Dir.listFiles()) - .filter(f -> f.getName().endsWith(".sst")).count(); + .filter(f -> f.getName().endsWith(SST_FILE_EXTENSION)).count(); long snap2SstFileCountBeforeFilter = Arrays.stream(snapshot2Dir.listFiles()) - .filter(f -> f.getName().endsWith(".sst")).count(); + .filter(f -> f.getName().endsWith(SST_FILE_EXTENSION)).count(); // delete snap1 - writeClient.deleteSnapshot(volumeName, bucketNames.get(0), "snap1"); + deleteSnapshot(volumeName, bucketNames.get(0), "snap1"); sstFilteringService.resume(); // Filtering service will only act on snap2 as it is an active snaphot with().atMost(Duration.ofSeconds(10)).pollInterval(Duration.ofSeconds(1)) .await() - .until(() -> sstFilteringService.getSnapshotFilteredCount().get() >= 2); + .until(() -> sstFilteringService.getSnapshotFilteredCount().get() >= countTotalSnapshots); long snap1SstFileCountAfterFilter = Arrays.stream(snapshot1Dir.listFiles()) .filter(f -> f.getName().endsWith(SST_FILE_EXTENSION)).count(); long snap2SstFileCountAfterFilter = Arrays.stream(snapshot2Dir.listFiles()) @@ -315,10 +324,12 @@ public void testActiveAndDeletedSnapshotCleanup() throws IOException { // one sst will be filtered in both active but not in deleted snapshot // as sstFiltering svc won't run on already deleted snapshots but will mark // it as filtered. - assertEquals(2, sstFilteringService.getSnapshotFilteredCount().get()); + assertEquals(countTotalSnapshots, sstFilteringService.getSnapshotFilteredCount().get()); assertEquals(snap1SstFileCountBeforeFilter, snap1SstFileCountAfterFilter); - assertEquals(snap2SstFileCountBeforeFilter - 1, - snap2SstFileCountAfterFilter); + // If method with order 1 is run .sst file from /vol1/buck1 and /vol1/buck2 will be deleted. + // As part of this method .sst file from /volume1/bucket2/ will be deleted. + // sstFiltering won't run on deleted snapshots in /volume1/bucket1. + assertThat(snap2SstFileCountBeforeFilter).isGreaterThan(snap2SstFileCountAfterFilter); } private void createKeys(String volumeName, @@ -331,8 +342,7 @@ private void createKeys(String volumeName, } } - private void createVolumeAndBucket(String volumeName, - String bucketName) + private void createVolume(String volumeName) throws IOException { OMRequestTestUtils.addVolumeToOM(keyManager.getMetadataManager(), OmVolumeArgs.newBuilder() @@ -340,7 +350,10 @@ private void createVolumeAndBucket(String volumeName, .setAdminName("a") .setVolume(volumeName) .build()); + } + private void addBucketToVolume(String volumeName, String bucketName) + throws IOException { OMRequestTestUtils.addBucketToOM(keyManager.getMetadataManager(), OmBucketInfo.newBuilder().setVolumeName(volumeName) .setBucketName(bucketName) @@ -381,13 +394,15 @@ private void createKey(OzoneManagerProtocol managerProtocol, * snapshot bucket. */ @Test + @Order(3) public void testSstFilteringService() throws IOException { RDBStore activeDbStore = (RDBStore) om.getMetadataManager().getStore(); String volumeName = "volume"; List bucketNames = Arrays.asList("bucket", "bucket1", "bucket2"); + createVolume(volumeName); for (String bucketName : bucketNames) { - createVolumeAndBucket(volumeName, bucketName); + addBucketToVolume(volumeName, bucketName); } int keyCount = 150; @@ -422,8 +437,7 @@ public void testSstFilteringService() throws IOException { List snapshotNames = Arrays.asList("snap", "snap-1", "snap-2"); for (int i = 0; i < 3; i++) { - writeClient.createSnapshot(volumeName, bucketNames.get(i), - snapshotNames.get(i)); + createSnapshot(volumeName, bucketNames.get(i), snapshotNames.get(i)); } SstFilteringService sstFilteringService = @@ -432,8 +446,8 @@ public void testSstFilteringService() throws IOException { with().atMost(Duration.ofSeconds(10)) .pollInterval(Duration.ofSeconds(1)) .await() - .until(() -> sstFilteringService.getSnapshotFilteredCount().get() >= 3); - assertEquals(3, sstFilteringService.getSnapshotFilteredCount().get()); + .until(() -> sstFilteringService.getSnapshotFilteredCount().get() >= countTotalSnapshots); + assertEquals(countTotalSnapshots, sstFilteringService.getSnapshotFilteredCount().get()); Set keyInBucketAfterFilteringRun = getKeysFromSnapshot(volumeName, bucketNames.get(0), @@ -483,4 +497,14 @@ private Set getKeysFromSnapshot(String volume, return getKeysFromDb(omSnapshot.getMetadataManager(), volume, bucket); } } + + private void createSnapshot(String volumeName, String bucketName, String snapshotName) throws IOException { + writeClient.createSnapshot(volumeName, bucketName, snapshotName); + countTotalSnapshots++; + } + + private void deleteSnapshot(String volumeName, String bucketName, String snapshotName) throws IOException { + writeClient.deleteSnapshot(volumeName, bucketName, snapshotName); + countTotalSnapshots--; + } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java new file mode 100644 index 000000000000..3948f4fab805 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDeletingService.java @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package org.apache.hadoop.ozone.om.service; + + +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; +import org.apache.hadoop.ozone.om.KeyManagerImpl; +import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; +import org.apache.hadoop.ozone.om.OmSnapshotManager; +import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.SnapshotChainManager; +import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.IOException; +import java.time.Duration; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Class to unit test SnapshotDeletingService. + */ +@ExtendWith(MockitoExtension.class) +public class TestSnapshotDeletingService { + @Mock + private OzoneManager ozoneManager; + @Mock + private KeyManagerImpl keyManager; + @Mock + private OmSnapshotManager omSnapshotManager; + @Mock + private SnapshotChainManager chainManager; + @Mock + private OmMetadataManagerImpl omMetadataManager; + @Mock + private ScmBlockLocationProtocol scmClient; + private final OzoneConfiguration conf = new OzoneConfiguration();; + private final long sdsRunInterval = Duration.ofMillis(1000).toMillis(); + private final long sdsServiceTimeout = Duration.ofSeconds(10).toMillis(); + + + private static Stream testCasesForIgnoreSnapshotGc() { + SnapshotInfo filteredSnapshot = SnapshotInfo.newBuilder().setSstFiltered(true).setName("snap1").build(); + SnapshotInfo unFilteredSnapshot = SnapshotInfo.newBuilder().setSstFiltered(false).setName("snap1").build(); + return Stream.of( + Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false), + Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true), + Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false), + Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true), + Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false), + Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false), + Arguments.of(unFilteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true), + Arguments.of(filteredSnapshot, SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true)); + } + + @ParameterizedTest + @MethodSource("testCasesForIgnoreSnapshotGc") + public void testProcessSnapshotLogicInSDS(SnapshotInfo snapshotInfo, + SnapshotInfo.SnapshotStatus status, + boolean expectedOutcome) + throws IOException { + Mockito.when(omMetadataManager.getSnapshotChainManager()).thenReturn(chainManager); + Mockito.when(ozoneManager.getOmSnapshotManager()).thenReturn(omSnapshotManager); + Mockito.when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager); + Mockito.when(ozoneManager.getConfiguration()).thenReturn(conf); + + SnapshotDeletingService snapshotDeletingService = + new SnapshotDeletingService(sdsRunInterval, sdsServiceTimeout, ozoneManager, scmClient); + + snapshotInfo.setSnapshotStatus(status); + assertEquals(expectedOutcome, snapshotDeletingService.shouldIgnoreSnapshot(snapshotInfo)); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java index da182730bc8f..7e2483e574b5 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java @@ -18,21 +18,15 @@ package org.apache.hadoop.ozone.om.snapshot; -import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; -import org.apache.hadoop.ozone.om.service.SnapshotDeletingService; import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; import java.io.File; import java.nio.file.Files; import java.nio.file.Path; import java.util.Set; import java.util.stream.Collectors; -import java.util.stream.Stream; import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils.getINode; @@ -84,42 +78,4 @@ public void testLinkFiles(@TempDir File tempDir) throws Exception { assertEquals(tree1Files, tree2Files); GenericTestUtils.deleteDirectory(tempDir); } - - - private static Stream testCasesForIgnoreSnapshotGc() { - SnapshotInfo filteredSnapshot = - SnapshotInfo.newBuilder().setSstFiltered(true).setName("snap1").build(); - SnapshotInfo unFilteredSnapshot = - SnapshotInfo.newBuilder().setSstFiltered(false).setName("snap1") - .build(); - // {IsSnapshotFiltered,isSnapshotDeleted,IsSstServiceEnabled = ShouldIgnore} - return Stream.of(Arguments.of(filteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, true, false), - Arguments.of(filteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true, true), - Arguments.of(unFilteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, true, true), - Arguments.of(unFilteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, true, true), - Arguments.of(filteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false, false), - Arguments.of(unFilteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED, false, false), - Arguments.of(unFilteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, false, true), - Arguments.of(filteredSnapshot, - SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE, false, true)); - } - - @ParameterizedTest - @MethodSource("testCasesForIgnoreSnapshotGc") - public void testProcessSnapshotLogicInSDS(SnapshotInfo snapshotInfo, - SnapshotInfo.SnapshotStatus status, boolean isSstFilteringSvcEnabled, - boolean expectedOutcome) { - snapshotInfo.setSnapshotStatus(status); - assertEquals(expectedOutcome, - SnapshotDeletingService.shouldIgnoreSnapshot(snapshotInfo, - isSstFilteringSvcEnabled)); - } - }