joshelser commented on code in PR #4419:
URL: https://github.com/apache/hbase/pull/4419#discussion_r869802361


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java:
##########
@@ -119,18 +119,18 @@ public class StoreFileInfo implements Configurable {
    */
   public StoreFileInfo(final Configuration conf, final FileSystem fs, final 
Path initialPath,
     final boolean primaryReplica) throws IOException {
-    this(conf, fs, null, initialPath, primaryReplica);
+    this(conf, fs, fs.getFileStatus(initialPath), primaryReplica);

Review Comment:
   Should make sure constructor javadoc is updated as this is a Namenode call. 
If we call this often, we're incurring remote reads which could be a bad perf 
hit. Need to make sure current callers are behaving nicely (we _should_ be 
caching StoreFileInfo's already)



##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java:
##########
@@ -940,13 +940,15 @@ private void archiveStoreFile(int index) throws 
IOException {
   private void closeCompactedFile(int index) throws IOException {
     Collection<HStoreFile> files =
       this.store.getStoreEngine().getStoreFileManager().getCompactedfiles();
-    HStoreFile sf = null;
-    Iterator<HStoreFile> it = files.iterator();
-    for (int i = 0; i <= index; i++) {
-      sf = it.next();
+    if (files.size() > 0) {
+      HStoreFile sf = null;
+      Iterator<HStoreFile> it = files.iterator();
+      for (int i = 0; i <= index; i++) {
+        sf = it.next();
+      }
+      sf.closeStoreFile(true);
+      
store.getStoreEngine().getStoreFileManager().removeCompactedFiles(Lists.newArrayList(sf));

Review Comment:
   nit: `Collections.singletonList(sf)` might be a little less heavy than a new 
ArrayList (since you're changing 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