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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to