[ 
https://issues.apache.org/jira/browse/HBASE-9992?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13826766#comment-13826766
 ] 

Jonathan Hsieh commented on HBASE-9992:
---------------------------------------

I don't see how this is hacky -- can you explain why. This approach now 
captures *all* cases where configuration params from a -D at the command line 
get put into the conf.  If you look at the v3 version of the patch at 
HBASE-9831. What I committed for HBASE-9831 only "caught" the specific case 
mentioned in the HBASE-9831 case.

Excluding the revert, here's the patch:

{code}
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/util
index 09e3acd..02c3b7a 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
@@ -164,7 +164,7 @@ import com.google.protobuf.ServiceException;
  */
 @InterfaceAudience.Public
 @InterfaceStability.Evolving
-public class HBaseFsck extends Configured implements Tool {
+public class HBaseFsck extends Configured {
   public static final long DEFAULT_TIME_LAG = 60000; // default value of 1 
minute
   public static final long DEFAULT_SLEEP_BEFORE_RERUN = 10000;
   private static final int MAX_NUM_THREADS = 50; // #threads to contact regions
@@ -3590,15 +3590,23 @@ public class HBaseFsck extends Configured implements 
Tool {
     URI defaultFs = hbasedir.getFileSystem(conf).getUri();
     FSUtils.setFsDefault(conf, new Path(defaultFs));
 
-    int ret = ToolRunner.run(new HBaseFsck(conf), args);
+    int ret = ToolRunner.run(new HBaseFsckTool(conf), args);
     System.exit(ret);
   }
 
-  @Override
-  public int run(String[] args) throws Exception {
-    exec(executor, args);
-    return getRetCode();
-  }
+  /**
+   * This is a Tool wrapper that gathers -Dxxx=yyy configuration settings from 
the command line.
+   */
+  static class HBaseFsckTool extends Configured implements Tool {
+    HBaseFsckTool(Configuration conf) { super(conf); }
+    @Override
+    public int run(String[] args) throws Exception {
+      HBaseFsck hbck = new HBaseFsck(getConf());
+      hbck.exec(hbck.executor, args);
+      return hbck.getRetCode();
+    }
+  };
+  
 
   public HBaseFsck exec(ExecutorService exec, String[] args) throws 
KeeperException, IOException,
     ServiceException, InterruptedException {
{code}

> [hbck] Refactor so that arbitrary -D cmdline options are included 
> ------------------------------------------------------------------
>
>                 Key: HBASE-9992
>                 URL: https://issues.apache.org/jira/browse/HBASE-9992
>             Project: HBase
>          Issue Type: Bug
>          Components: hbck
>    Affects Versions: 0.96.0, 0.94.13
>            Reporter: Jonathan Hsieh
>            Assignee: Jonathan Hsieh
>             Fix For: 0.98.0, 0.96.1, 0.94.14
>
>         Attachments: hbase-9992.patch
>
>
> A review of HBASE-9831 pointed out the fact that -D options aren't being 
> passed into the configuration object used by hbck.  This means overriding -D 
> options will not work unless special hooks are for specific options.  A first 
> attempt to fix this was in HBASE-9831 but it affected many other files.
> The right approach would be to create a new HbckTool class that had the 
> configured interface and change to existing HBaseFsck main to instantiate 
> that to have it parse args, and then create the HBaseFsck object inside run.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to