hemantk-12 commented on code in PR #6965:
URL: https://github.com/apache/ozone/pull/6965#discussion_r1698950352


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java:
##########
@@ -112,33 +126,34 @@ 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.
       OMLockDetails omLockDetails = ozoneManager.getMetadataManager().getLock()
-              .acquireWriteLock(SNAPSHOT_LOCK, volume, bucket, snapshotName);
+          .acquireReadLock(SNAPSHOT_LOCK, snapshotInfo.getVolumeName(), 
snapshotInfo.getBucketName(),

Review Comment:
   nit: It doesn't matter if it is a read lock or write lock from the 
correctness perspective. But since we are writing to dir, it should be a write 
lock as it is in `OMSnapshotPurgeResponse`.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java:
##########
@@ -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");

Review Comment:
   ```suggestion
         "this file unless you know what you are doing.\n");
   ```



-- 
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