HDDS-11068. Move SstFiltered flag to a file in the snapshot directory #6965
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,10 +20,11 @@ | |
|
|
||
| import org.apache.commons.io.FileUtils; | ||
| import org.apache.hadoop.hdds.utils.db.BatchOperation; | ||
| 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 jakarta.annotation.Nonnull; | ||
| import java.io.IOException; | ||
| import java.nio.file.Path; | ||
| 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. | ||
|
|
@@ -116,15 +117,23 @@ private void updateSnapInfo(OmMetadataManagerImpl metadataManager, | |
| */ | ||
| private void deleteCheckpointDirectory(OMMetadataManager omMetadataManager, | ||
| SnapshotInfo snapshotInfo) { | ||
| // Acquiring write lock to avoid race condition with sst filtering service which creates a sst filtered file | ||
| // inside the snapshot directory. Any operation apart from delete can run in parallel along with operation. | ||
hemantk-12 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 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()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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.OmSnapshot; | ||
| import org.apache.hadoop.ozone.om.OmSnapshotManager; | ||
| import org.apache.hadoop.ozone.om.OzoneManager; | ||
|
|
@@ -100,7 +99,6 @@ public class SnapshotDeletingService extends AbstractKeyDeletingService { | |
| private final long snapshotDeletionPerTask; | ||
| private final int keyLimitPerSnapshot; | ||
| private final int ratisByteLimit; | ||
|
|
||
| public SnapshotDeletingService(long interval, long serviceTimeout, | ||
| OzoneManager ozoneManager, ScmBlockLocationProtocol scmClient) | ||
|
|
@@ -128,8 +126,6 @@ public SnapshotDeletingService(long interval, long serviceTimeout, | |
| this.keyLimitPerSnapshot = conf.getInt( | ||
| OZONE_SNAPSHOT_KEY_DELETING_LIMIT_PER_TASK, | ||
| OZONE_SNAPSHOT_KEY_DELETING_LIMIT_PER_TASK_DEFAULT); | ||
| } | ||
|
|
||
| private class SnapshotDeletingTask implements BackgroundTask { | ||
|
|
@@ -594,8 +590,7 @@ public void submitRequest(OMRequest omRequest) { | |
| @VisibleForTesting | ||
| boolean shouldIgnoreSnapshot(SnapshotInfo snapInfo) { | ||
| SnapshotInfo.SnapshotStatus snapshotStatus = snapInfo.getSnapshotStatus(); | ||
| return snapshotStatus != SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED; | ||
| } | ||
|
|
||
| // TODO: Move this util class. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,26 +67,23 @@ private static Stream<Arguments> 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); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,7 +206,7 @@ public void testIrrelevantSstFileDeletion() | |
| createSnapshot(volumeName, bucketName2, snapshotName1); | ||
| SnapshotInfo snapshotInfo = om.getMetadataManager().getSnapshotInfoTable() | ||
| .get(SnapshotInfo.getTableKey(volumeName, bucketName2, snapshotName1)); | ||
| assertFalse(SstFilteringService.isSstFiltered(om.getConfiguration(), snapshotInfo)); | ||
| waitForSnapshotsAtLeast(filteringService, countExistingSnapshots + 1); | ||
| assertEquals(countExistingSnapshots + 1, filteringService.getSnapshotFilteredCount().get()); | ||
|
|
||
|
|
@@ -238,8 +238,9 @@ public void testIrrelevantSstFileDeletion() | |
|
|
||
| // 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"; | ||
| final long count; | ||
|
|
@@ -313,7 +314,7 @@ public void testActiveAndDeletedSnapshotCleanup() throws Exception { | |
| .filter(f -> f.getName().endsWith(SST_FILE_EXTENSION)).count(); | ||
|
|
||
| // delete snap1 | ||
| deleteSnapshot(volumeName, bucketNames.get(0), "snap1"); | ||
| sstFilteringService.resume(); | ||
| // Filtering service will only act on snap2 as it is an active snaphot | ||
| waitForSnapshotsAtLeast(sstFilteringService, countTotalSnapshots); | ||
|
|
@@ -505,4 +506,9 @@ private void createSnapshot(String volumeName, String bucketName, String snapsho | |
| writeClient.createSnapshot(volumeName, bucketName, snapshotName); | ||
| countTotalSnapshots++; | ||
| } | ||
|
|
||
| private void deleteSnapshot(String volumeName, String bucketName, String snapshotName) throws IOException { | ||
| writeClient.deleteSnapshot(volumeName, bucketName, snapshotName); | ||
| countTotalSnapshots--; | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we have
.txtor any other text file extension?