apurtell commented on code in PR #4403:
URL: https://github.com/apache/hbase/pull/4403#discussion_r876494765


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java:
##########
@@ -88,6 +88,24 @@ public HFileCleaner(final int period, final Stoppable 
stopper, Configuration con
     "hbase.regionserver.hfilecleaner.thread.check.interval.msec";
   static final long DEFAULT_HFILE_DELETE_THREAD_CHECK_INTERVAL_MSEC = 1000L;
 
+  /**
+   * Configuration to enable custom hfile paths (under archive) for cleaner, 
which can use the
+   * independent pool and configuration.
+   */
+  public static final String HFILE_CLEANER_ENABLE_CUSTOM_PATHS =

Review Comment:
   We don't need this. If the user wants to configure this option they can 
simply set `HFILE_CLEANER_CUSTOM_PATHS` . If unset or the empty string we know 
it does not need to be enabled. If set and not an empty string, we know it 
should be enabled.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -384,6 +387,9 @@ public class HMaster extends 
HBaseServerBase<MasterRpcServices> implements Maste
   private DirScanPool logCleanerPool;
   private LogCleaner logCleaner;
   private HFileCleaner hfileCleaner;
+  private HFileCleaner[] customHFileCleaners;
+  private Path[] customHFilePaths;
+  private DirScanPool customHFileCleanerPool;

Review Comment:
   Same idea here, one unified `List` of `DirScanPool`s.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -384,6 +387,9 @@ public class HMaster extends 
HBaseServerBase<MasterRpcServices> implements Maste
   private DirScanPool logCleanerPool;
   private LogCleaner logCleaner;
   private HFileCleaner hfileCleaner;
+  private HFileCleaner[] customHFileCleaners;
+  private Path[] customHFilePaths;

Review Comment:
   Same idea here, why not just one array of hfile paths where each element 
corresponds with same index of element in cleaner array (or is null)?



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -384,6 +387,9 @@ public class HMaster extends 
HBaseServerBase<MasterRpcServices> implements Maste
   private DirScanPool logCleanerPool;
   private LogCleaner logCleaner;
   private HFileCleaner hfileCleaner;
+  private HFileCleaner[] customHFileCleaners;

Review Comment:
   Can we just do an array or List of all `HFileCleaner`s ? Is there a reason 
to manage `hfileCleaner` and `customHFileCleaners` separately?



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