Apache9 commented on code in PR #5545:
URL: https://github.com/apache/hbase/pull/5545#discussion_r1601087841


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java:
##########
@@ -48,17 +52,38 @@ class DefaultStoreFileManager implements StoreFileManager {
   private final CompactionConfiguration comConf;
   private final int blockingFileCount;
   private final Comparator<HStoreFile> storeFileComparator;
-  /**
-   * List of store files inside this store. This is an immutable list that is 
atomically replaced
-   * when its contents change.
-   */
-  private volatile ImmutableList<HStoreFile> storefiles = ImmutableList.of();
+
+  static class StoreFileList {
+    /**
+     * List of store files inside this store. This is an immutable list that 
is atomically replaced
+     * when its contents change.
+     */
+    final ImmutableList<HStoreFile> all;
+    /**
+     * List of store files that include the latest cells inside this store. 
This is an immutable
+     * list that is atomically replaced when its contents change.
+     */
+    @Nullable
+    final ImmutableList<HStoreFile> live;
+
+    StoreFileList(ImmutableList<HStoreFile> storeFiles, 
ImmutableList<HStoreFile> liveStoreFiles) {
+      this.all = storeFiles;
+      this.live = liveStoreFiles;
+    }
+  }
+
+  private static final StoreFileList EMPTY_STORE_FILE_LIST =
+    new StoreFileList(ImmutableList.of(), null);
+
+  private volatile StoreFileList storeFiles = EMPTY_STORE_FILE_LIST;

Review Comment:
   When initializating we do not need to test whether live file tracking is 
enabled? I see in later code, if live file tracking is enabled we will pass 
ImmutableList.of() instead of null here.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileManager.java:
##########
@@ -50,15 +50,15 @@ public interface StoreFileManager {
    */
   @RestrictedApi(explanation = "Should only be called in StoreEngine", link = 
"",
       allowedOnPath = 
".*(/org/apache/hadoop/hbase/regionserver/StoreEngine.java|/src/test/.*)")
-  void loadFiles(List<HStoreFile> storeFiles);
+  void loadFiles(List<HStoreFile> storeFiles) throws IOException;

Review Comment:
   I know the there are throws declarations, what I mean here is whether it 
will actually throw any exceptions out. For example, in loadFiles, maybe we 
have already called initReader for all files so there is no problem? Just want 
to confirm this.



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

Reply via email to