[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4633: HDDS-7546. [Snapshot] Throw exception if the SnapshotDiff size is huge

["hemantk-12 (via GitHub)" <gi...@apache.org>]

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


##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/ManagedSstFileReader.java:
##########
@@ -49,16 +49,37 @@ public class ManagedSstFileReader {
 
   private final Collection<String> sstFiles;
 
+  private volatile long estimatedTotalKeys = -1;
+
   public ManagedSstFileReader(final Collection<String> sstFiles) {
     this.sstFiles = sstFiles;
   }
 
   public static <T> Stream<T> getStreamFromIterator(ClosableIterator<T> itr) {
     final Spliterator<T> spliterator =
         Spliterators.spliteratorUnknownSize(itr, 0);
-    return StreamSupport.stream(spliterator, false).onClose(() -> {
-      itr.close();
-    });
+    return StreamSupport.stream(spliterator, false).onClose(itr::close);
+  }
+
+  public long getEstimatedTotalKeys() throws RocksDBException {
+    if (estimatedTotalKeys != -1) {
+      return estimatedTotalKeys;
+    }
+
+    long estimatedSize = 0;
+    synchronized (this) {
+      try (ManagedOptions options = new ManagedOptions()) {
+        for (String sstFile : sstFiles) {
+          SstFileReader fileReader = new SstFileReader(options);
+          fileReader.open(sstFile);
+          estimatedSize += fileReader.getTableProperties().getNumEntries();
+          estimatedSize += fileReader.getTableProperties().getNumDeletions();
+        }
+      }
+    }
+
+    estimatedTotalKeys = estimatedSize;

Review Comment:
   1. It makes sense to move `estimatedTotalKeys = estimatedSize;` this in synchronized block. Will update it.
   2. Regarding having double check `if (estimatedTotalKeys != -1)` inside the synchronized block, I agree but [findbug failed](https://github.com/apache/ozone/actions/runs/4813599053/jobs/8571917550) once when I did something similar to create singleton object. Another reason is that it doesn't matter that much because result is used by same thread. I'll add the check and see if findbug works. If does I'll update in next revision otherwise leave as it is.
   



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