You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2023/08/07 18:13:28 UTC
[GitHub] [ozone] swamirishi commented on a diff in pull request #5155: HDDS-9126. [Snapshot] SstFilteringService should only process active snapshots.
swamirishi commented on code in PR #5155:
URL: https://github.com/apache/ozone/pull/5155#discussion_r1286232742
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java:
##########
@@ -149,49 +161,55 @@ public BackgroundTaskResult call() throws Exception {
Table.KeyValue<String, SnapshotInfo> keyValue = iterator.next();
String snapShotTableKey = keyValue.getKey();
SnapshotInfo snapshotInfo = keyValue.getValue();
- UUID snapshotId = snapshotInfo.getSnapshotId();
-
- File omMetadataDir =
- OMStorage.getOmDbDir(ozoneManager.getConfiguration());
- String snapshotDir = omMetadataDir + OM_KEY_PREFIX + OM_SNAPSHOT_DIR;
- Path filePath =
- Paths.get(snapshotDir + OM_KEY_PREFIX + FILTERED_SNAPSHOTS);
-
- // If entry for the snapshotID is present in this file,
- // it has already undergone filtering.
- if (Files.exists(filePath)) {
- List<String> processedSnapshotIds = Files.readAllLines(filePath);
- if (snapshotId != null &&
- processedSnapshotIds.contains(snapshotId.toString())) {
- continue;
+ if (snapshotInfo.getSnapshotStatus()
+ .equals(SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE)) {
+ UUID snapshotId = snapshotInfo.getSnapshotId();
+
+ File omMetadataDir =
+ OMStorage.getOmDbDir(ozoneManager.getConfiguration());
+ String snapshotDir =
+ omMetadataDir + OM_KEY_PREFIX + OM_SNAPSHOT_DIR;
+ Path filePath =
+ Paths.get(snapshotDir + OM_KEY_PREFIX + FILTERED_SNAPSHOTS);
+
+ // If entry for the snapshotID is present in this file,
+ // it has already undergone filtering.
+ if (Files.exists(filePath)) {
+ List<String> processedSnapshotIds = Files.readAllLines(filePath);
+ if (snapshotId != null && processedSnapshotIds.contains(
+ snapshotId.toString())) {
+ continue;
+ }
}
- }
- LOG.debug("Processing snapshot {} to filter relevant SST Files",
- snapShotTableKey);
-
- List<Pair<String, String>> prefixPairs =
- constructPrefixPairs(snapshotInfo);
-
- try (ReferenceCounted<IOmMetadataReader, SnapshotCache>
- snapshotMetadataReader = snapshotCache.get()
- .get(snapshotInfo.getTableKey())) {
- OmSnapshot omSnapshot = (OmSnapshot) snapshotMetadataReader.get();
- RDBStore rdbStore = (RDBStore) omSnapshot.getMetadataManager()
- .getStore();
- RocksDatabase db = rdbStore.getDb();
- try (BootstrapStateHandler.Lock lock =
- getBootstrapStateLock().lock()) {
- db.deleteFilesNotMatchingPrefix(prefixPairs, FILTER_FUNCTION);
+ LOG.debug("Processing snapshot {} to filter relevant SST Files",
+ snapShotTableKey);
+
+ List<Pair<String, String>> prefixPairs =
+ constructPrefixPairs(snapshotInfo);
+
+ try (
+ ReferenceCounted<IOmMetadataReader, SnapshotCache>
+ snapshotMetadataReader = snapshotCache.get().get(
+ snapshotInfo.getTableKey())) {
Review Comment:
There is no locking mechanism here. Snapshot could have been marked inactive b/w line 161 & line 194. Failure can still occur. We can probably run the sstFiltering service with skipActiveCheck as True instead.
```suggestion
snapshotInfo.getTableKey(), true)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org