You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by te...@apache.org on 2016/09/14 14:34:41 UTC

hbase git commit: HBASE-16620 Fix backup command-line tool usability issues (Vladimir Rodionov)

Repository: hbase
Updated Branches:
  refs/heads/HBASE-7912 44812cf1e -> e5d77d095


HBASE-16620 Fix backup command-line tool usability issues (Vladimir Rodionov)


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e5d77d09
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e5d77d09
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e5d77d09

Branch: refs/heads/HBASE-7912
Commit: e5d77d095c4f726c2cbaef25be36ff547c07126d
Parents: 44812cf
Author: tedyu <yu...@gmail.com>
Authored: Wed Sep 14 07:34:11 2016 -0700
Committer: tedyu <yu...@gmail.com>
Committed: Wed Sep 14 07:34:11 2016 -0700

----------------------------------------------------------------------
 .../hbase/backup/impl/BackupCommands.java       | 275 ++++++++++++-------
 .../hadoop/hbase/backup/BackupDriver.java       |  71 ++++-
 .../hadoop/hbase/backup/RestoreDriver.java      |  75 ++++-
 .../hadoop/hbase/util/AbstractHBaseTool.java    |  10 +-
 .../hbase/backup/TestBackupCommandLineTool.java | 175 ++++++++++++
 .../hadoop/hbase/backup/TestFullBackupSet.java  |   2 +-
 6 files changed, 490 insertions(+), 118 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/e5d77d09/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java
index 8a4d48a..31aa348 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java
@@ -49,66 +49,80 @@ import com.google.common.collect.Lists;
 @InterfaceAudience.Private
 @InterfaceStability.Evolving
 public final class BackupCommands {
+  
+  public final static String IGNORE = "ignore";
 
-  private static final String USAGE = "Usage: hbase backup COMMAND\n"
+  public static final String USAGE = "Usage: hbase backup COMMAND [command-specific arguments]\n"
       + "where COMMAND is one of:\n" 
       + "  create     create a new backup image\n"
-      + "  cancel     cancel an ongoing backup\n"
       + "  delete     delete an existing backup image\n"
       + "  describe   show the detailed information of a backup image\n"
       + "  history    show history of all successful backups\n"
       + "  progress   show the progress of the latest backup request\n"
       + "  set        backup set management\n"
-      + "Enter \'help COMMAND\' to see help message for each command\n";
-
-  private static final String CREATE_CMD_USAGE =
-      "Usage: hbase backup create <type> <BACKUP_ROOT> [tables] [-s name] [-convert] "
-          + "[-silent] [-w workers][-b bandwith]\n" 
-          + " type          \"full\" to create a full backup image;\n"
-          + "               \"incremental\" to create an incremental backup image\n"
-          + "  BACKUP_ROOT   The full root path to store the backup image,\n"
-          + "                    the prefix can be hdfs, webhdfs or gpfs\n" + " Options:\n"
-          + "  tables      If no tables (\"\") are specified, all tables are backed up. "
-          + "Otherwise it is a\n" + "               comma separated list of tables.\n"
-          + " -w          number of parallel workers.\n" 
-          + " -b          bandwith per one worker (in MB sec)\n" 
-          + " -set        name of backup set" ;
-
-  private static final String PROGRESS_CMD_USAGE = "Usage: hbase backup progress <backupId>\n"
-      + " backupId      backup image id;\n";
-
-  private static final String DESCRIBE_CMD_USAGE = "Usage: hbase backup decsribe <backupId>\n"
-      + " backupId      backup image id\n";
-
-  private static final String HISTORY_CMD_USAGE = 
+      + "Run \'hbase backup help COMMAND\' to see help message for each command\n";
+
+  public static final String CREATE_CMD_USAGE =
+      "Usage: hbase backup create <type> <BACKUP_ROOT> [tables] [-set name] "
+          + "[-w workers][-b bandwith]\n" 
+          + " type           \"full\" to create a full backup image\n"
+          + "                \"incremental\" to create an incremental backup image\n"
+          + " BACKUP_ROOT     The full root path to store the backup image,\n"
+          + "                 the prefix can be hdfs, webhdfs or gpfs\n" 
+          + "Options:\n"
+          + " tables          If no tables (\"\") are specified, all tables are backed up.\n"
+          + "                 Otherwise it is a comma separated list of tables.\n"
+          + " -w              number of parallel workers (MapReduce tasks).\n" 
+          + " -b              bandwith per one worker (MapReduce task) in MBs per sec\n" 
+          + " -set            name of backup set to use (mutually exclusive with [tables])" ;
+
+  public static final String PROGRESS_CMD_USAGE = "Usage: hbase backup progress <backupId>\n"
+          + " backupId        backup image id\n";
+
+  public static final String DESCRIBE_CMD_USAGE = "Usage: hbase backup decsribe <backupId>\n"
+          + " backupId        backup image id\n";
+
+  public static final String HISTORY_CMD_USAGE = 
       "Usage: hbase backup history [-path BACKUP_ROOT] [-n N] [-t table]\n"
-      + " -n N     show up to N last backup sessions, default - 10;\n"
-      + " -path    backup root path;\n"
-      + " -t       table name; ";
+       + " -n N            show up to N last backup sessions, default - 10\n"
+       + " -path           backup root path\n"
+       + " -t table        table name. If specified, only backup images which contain this table\n"
+       + "                 will be listed."  ;
   
 
-  private static final String DELETE_CMD_USAGE = "Usage: hbase backup delete <backupId>\n"
-      + " backupId      backup image id;\n";
+  public static final String DELETE_CMD_USAGE = "Usage: hbase backup delete <backupId>\n"
+          + " backupId        backup image id\n";
 
-  private static final String CANCEL_CMD_USAGE = "Usage: hbase backup cancel <backupId>\n"
-      + " backupId      backup image id;\n";
+  public static final String CANCEL_CMD_USAGE = "Usage: hbase backup cancel <backupId>\n"
+          + " backupId        backup image id\n";
 
-  private static final String SET_CMD_USAGE = "Usage: hbase backup set COMMAND [name] [tables]\n"
-      + " name       Backup set name\n"
-      + " tables      If no tables (\"\") are specified, all tables will belong to the set. "
-      + "Otherwise it is a\n" + "               comma separated list of tables.\n"
-      + "where COMMAND is one of:\n" 
-      + "  add      add tables to a set, crete set if needed\n"
-      + "  remove   remove tables from set\n"
-      + "  list     list all sets\n"
-      + "  describe describes set\n"
-      + "  delete   delete backup set\n";
+  public static final String SET_CMD_USAGE = "Usage: hbase backup set COMMAND [name] [tables]\n"
+         + " name            Backup set name\n"
+         + " tables          If no tables (\"\") are specified, all tables will belong to the set.\n"
+         + "                 Otherwise it is a comma separated list of tables.\n"
+         + "COMMAND is one of:\n" 
+         + " add             add tables to a set, create a set if needed\n"
+         + " remove          remove tables from a set\n"
+         + " list            list all backup sets in the system\n"
+         + " describe        describe set\n"
+         + " delete          delete backup set\n";
 
   public static abstract class Command extends Configured {
+    CommandLine cmdline;
+    
     Command(Configuration conf) {
       super(conf);
     }
-    public abstract void execute() throws IOException;
+    
+    public void execute() throws IOException
+    {
+      if (cmdline.hasOption("h") || cmdline.hasOption("help")) {
+        printUsage();
+        throw new IOException(IGNORE);
+      }
+    }
+    
+    protected abstract void printUsage();
   }
 
   private BackupCommands() {
@@ -149,7 +163,6 @@ public final class BackupCommands {
 
 
   public static class CreateCommand extends Command {
-    CommandLine cmdline;
 
     CreateCommand(Configuration conf, CommandLine cmdline) {
       super(conf);
@@ -158,25 +171,26 @@ public final class BackupCommands {
     
     @Override
     public void execute() throws IOException {
+      super.execute();
       if (cmdline == null || cmdline.getArgs() == null) {
-        System.out.println("ERROR: missing arguments");
-        System.out.println(CREATE_CMD_USAGE);
+        System.err.println("ERROR: missing arguments");
+        printUsage();
         System.exit(-1);
       }
       String[] args = cmdline.getArgs();
       if (args.length < 3 || args.length > 4) {
-        System.out.println("ERROR: wrong number of arguments");
-        System.out.println(CREATE_CMD_USAGE);
+        System.err.println("ERROR: wrong number of arguments: "+ args.length);
+        printUsage();
         System.exit(-1);
       }
 
       if (!BackupType.FULL.toString().equalsIgnoreCase(args[1])
           && !BackupType.INCREMENTAL.toString().equalsIgnoreCase(args[1])) {
-        System.out.println("ERROR: invalid backup type");
-        System.out.println(CREATE_CMD_USAGE);
+        System.err.println("ERROR: invalid backup type: "+ args[1]);
+        printUsage();
         System.exit(-1);
       }
-
+            
       String tables = null;
       Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create();
 
@@ -185,8 +199,11 @@ public final class BackupCommands {
         String setName = cmdline.getOptionValue("set");
         tables = getTablesForSet(setName, conf);
 
-        if (tables == null) throw new IOException("Backup set '" + setName
-          + "' is either empty or does not exist");
+        if (tables == null) {
+          System.err.println("ERROR: Backup set '" + setName+ "' is either empty or does not exist");
+          printUsage();
+          throw new IOException(IGNORE);
+        }
       } else {
         tables = (args.length == 4) ? args[3] : null;
       }
@@ -216,10 +233,14 @@ public final class BackupCommands {
         return StringUtils.join(tables, BackupRestoreConstants.TABLENAME_DELIMITER_IN_COMMAND);        
       }
     }
+
+    @Override
+    protected void printUsage() {
+      System.err.println(CREATE_CMD_USAGE);      
+    }
   }
 
   private static class HelpCommand extends Command {
-    CommandLine cmdline;
 
     HelpCommand(Configuration conf, CommandLine cmdline) {
       super(conf);
@@ -228,21 +249,22 @@ public final class BackupCommands {
 
     @Override
     public void execute() throws IOException {
+      super.execute();
       if (cmdline == null) {
-        System.out.println(USAGE);
-        System.exit(0);
+        printUsage();
+        System.exit(-1);
       }
 
       String[] args = cmdline.getArgs();
       if (args == null || args.length == 0) {
-        System.out.println(USAGE);
-        System.exit(0);
+        printUsage();
+        System.exit(-1);
       }
 
       if (args.length != 2) {
-        System.out.println("Only support check help message of a single command type");
-        System.out.println(USAGE);
-        System.exit(0);
+        System.err.println("Only supports help message of a single command type");
+        printUsage();
+        System.exit(-1);
       }
 
       String type = args[1];
@@ -263,14 +285,18 @@ public final class BackupCommands {
         System.out.println(SET_CMD_USAGE);
       } else {
         System.out.println("Unknown command : " + type);
-        System.out.println(USAGE);
+        printUsage();
       }
       System.exit(0);
     }
+
+    @Override
+    protected void printUsage() {
+      System.err.println(USAGE);      
+    }
   }
 
   private static class DescribeCommand extends Command {
-    CommandLine cmdline;
 
     DescribeCommand(Configuration conf, CommandLine cmdline) {
       super(conf);
@@ -279,18 +305,19 @@ public final class BackupCommands {
 
     @Override
     public void execute() throws IOException {
+      super.execute();
       if (cmdline == null || cmdline.getArgs() == null) {
-        System.out.println("ERROR: missing arguments");
-        System.out.println(DESCRIBE_CMD_USAGE);
+        System.err.println("ERROR: missing arguments");
+        printUsage();
         System.exit(-1);
       }
       String[] args = cmdline.getArgs();
       if (args.length != 2) {
-        System.out.println("ERROR: wrong number of arguments");
-        System.out.println(DESCRIBE_CMD_USAGE);
+        System.err.println("ERROR: wrong number of arguments");
+        printUsage();
         System.exit(-1);
       }
-
+            
       String backupId = args[1];
       Configuration conf = getConf() != null ? getConf() : HBaseConfiguration.create();
       try (final Connection conn = ConnectionFactory.createConnection(conf);
@@ -299,10 +326,14 @@ public final class BackupCommands {
         System.out.println(info.getShortDescription());
       }
     }
+
+    @Override
+    protected void printUsage() {
+      System.err.println(DESCRIBE_CMD_USAGE);      
+    }
   }
 
   private static class ProgressCommand extends Command {
-    CommandLine cmdline;
 
     ProgressCommand(Configuration conf, CommandLine cmdline) {
       super(conf);
@@ -311,6 +342,8 @@ public final class BackupCommands {
 
     @Override
     public void execute() throws IOException {
+      super.execute();
+      
       if (cmdline == null || cmdline.getArgs() == null ||
           cmdline.getArgs().length != 2) {
         System.out.println("No backup id was specified, "
@@ -318,11 +351,12 @@ public final class BackupCommands {
       }
       String[] args = cmdline.getArgs();
       if (args.length > 2) {
-        System.out.println("ERROR: wrong number of arguments: " + args.length);
-        System.out.println(PROGRESS_CMD_USAGE);
+        System.err.println("ERROR: wrong number of arguments: " + args.length);
+        printUsage();
         System.exit(-1);
       }
-
+      
+      
       String backupId = args == null ? null : args[1];
       Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create();
       try(final Connection conn = ConnectionFactory.createConnection(conf); 
@@ -335,11 +369,15 @@ public final class BackupCommands {
         }
       } 
     }
+
+    @Override
+    protected void printUsage() {
+      System.err.println(PROGRESS_CMD_USAGE);      
+    }
   }
 
   private static class DeleteCommand extends Command {
     
-    CommandLine cmdline;
     DeleteCommand(Configuration conf, CommandLine cmdline) {
       super(conf);
       this.cmdline = cmdline;
@@ -347,11 +385,13 @@ public final class BackupCommands {
 
     @Override
     public void execute() throws IOException {
+      super.execute();
       if (cmdline == null || cmdline.getArgs() == null || cmdline.getArgs().length < 2) {
-        System.out.println("No backup id(s) was specified");
-        System.out.println(PROGRESS_CMD_USAGE);
+        System.err.println("No backup id(s) was specified");
+        printUsage();
         System.exit(-1);
       }
+            
       String[] args = cmdline.getArgs();
 
       String[] backupIds = new String[args.length - 1];
@@ -364,12 +404,16 @@ public final class BackupCommands {
       }
 
     }
+
+    @Override
+    protected void printUsage() {
+      System.err.println(DELETE_CMD_USAGE);      
+    }
   }
 
 // TODO Cancel command  
   
   private static class CancelCommand extends Command {
-    CommandLine cmdline;
 
     CancelCommand(Configuration conf, CommandLine cmdline) {
       super(conf);
@@ -378,6 +422,7 @@ public final class BackupCommands {
 
     @Override
     public void execute() throws IOException {
+      super.execute();
       if (cmdline == null || cmdline.getArgs() == null || cmdline.getArgs().length < 2) {
         System.out.println("No backup id(s) was specified, will use the most recent one");
       }
@@ -389,10 +434,14 @@ public final class BackupCommands {
         // TODO cancel backup
       }
     }
+
+    @Override
+    protected void printUsage() {
+    }
   }
 
   private static class HistoryCommand extends Command {
-    CommandLine cmdline;
+    
     private final static int DEFAULT_HISTORY_LENGTH = 10;
     
     HistoryCommand(Configuration conf, CommandLine cmdline) {
@@ -403,6 +452,8 @@ public final class BackupCommands {
     @Override
     public void execute() throws IOException {
 
+      super.execute();
+      
       int n = parseHistoryLength();
       TableName tableName = getTableName();
       Path backupRootPath = getBackupRootPath();
@@ -424,9 +475,17 @@ public final class BackupCommands {
     }
     
     private Path getBackupRootPath() {
-      String value = cmdline.getOptionValue("path");
-      if (value == null) return null;
-      return new Path(value);
+      String value = null;
+      try{
+        value = cmdline.getOptionValue("path");
+        if (value == null) return null;
+        return new Path(value);
+      } catch (IllegalArgumentException e) {
+        System.err.println("ERROR: Illegal argument for backup root path: "+ value);
+        printUsage();
+        System.exit(-1);
+      }
+      return null;
     }
 
     private TableName getTableName() {
@@ -435,15 +494,29 @@ public final class BackupCommands {
       try{
         return TableName.valueOf(value);
       } catch (IllegalArgumentException e){
-        System.out.println("Illegal argument: "+ value);
-        return null;
+        System.err.println("Illegal argument for table name: "+ value);
+        printUsage();
+        System.exit(-1);
       }
+      return null;
     }
 
     private int parseHistoryLength() {
       String value = cmdline.getOptionValue("n");
-      if (value == null) return DEFAULT_HISTORY_LENGTH;
-      return Integer.parseInt(value);
+      try{
+        if (value == null) return DEFAULT_HISTORY_LENGTH;
+        return Integer.parseInt(value);
+      } catch(NumberFormatException e) {
+        System.err.println("Illegal argument for history length: "+ value);
+        printUsage();
+        System.exit(-1);
+      }
+      return 0;
+    }
+
+    @Override
+    protected void printUsage() {
+      System.err.println(HISTORY_CMD_USAGE);      
     }
   }
 
@@ -454,8 +527,6 @@ public final class BackupCommands {
     private final static String SET_DESCRIBE_CMD = "describe";
     private final static String SET_LIST_CMD = "list";
 
-    CommandLine cmdline;
-
     BackupSetCommand(Configuration conf, CommandLine cmdline) {
       super(conf);
       this.cmdline = cmdline;
@@ -463,11 +534,14 @@ public final class BackupCommands {
 
     @Override
     public void execute() throws IOException {
-
+      super.execute();      
       // Command-line must have at least one element
       if (cmdline == null || cmdline.getArgs() == null || cmdline.getArgs().length < 2) {
-        throw new IOException("command line format");
+        System.err.println("ERROR: Command line format");
+        printUsage();
+        System.exit(-1);
       }
+            
       String[] args = cmdline.getArgs();
       String cmdStr = args[1];
       BackupCommand cmd = getCommand(cmdStr);
@@ -509,7 +583,9 @@ public final class BackupCommands {
 
     private void processSetDescribe(String[] args) throws IOException {
       if (args == null || args.length != 3) {
-        throw new RuntimeException("Wrong number of args: "+args.length);
+        System.err.println("ERROR: Wrong number of args for 'set describe' command: "+args.length);
+        printUsage();
+        System.exit(-1);        
       }
       String setName = args[2];
       Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create();
@@ -526,7 +602,9 @@ public final class BackupCommands {
 
     private void processSetDelete(String[] args) throws IOException {
       if (args == null || args.length != 3) {
-        throw new RuntimeException("Wrong number of args");
+        System.err.println("ERROR: Wrong number of args for 'set delete' command: "+args.length);
+        printUsage();
+        System.exit(-1);
       }
       String setName = args[2];
       Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create();
@@ -543,8 +621,11 @@ public final class BackupCommands {
 
     private void processSetRemove(String[] args) throws IOException {
       if (args == null || args.length != 4) {
-        throw new RuntimeException("Wrong args");
+        System.err.println("ERROR: Wrong number of args for 'set remove' command: "+args.length);
+        printUsage();
+        System.exit(-1); 
       }
+      
       String setName = args[2];
       String[] tables = args[3].split(",");
       Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create();
@@ -556,7 +637,9 @@ public final class BackupCommands {
 
     private void processSetAdd(String[] args) throws IOException {
       if (args == null || args.length != 4) {
-        throw new RuntimeException("Wrong args");
+        System.err.println("ERROR: Wrong number of args for 'set add' command: "+args.length);
+        printUsage();
+        System.exit(-1); 
       }
       String setName = args[2];
       String[] tables = args[3].split(",");
@@ -584,9 +667,17 @@ public final class BackupCommands {
       } else if (cmdStr.equals(SET_LIST_CMD)) {
         return BackupCommand.SET_LIST;
       } else {
-        throw new IOException("Unknown command for 'set' :" + cmdStr);
+        System.err.println("ERROR: Unknown command for 'set' :" + cmdStr);
+        printUsage();
+        System.exit(-1); 
+        return null;
       }
     }
 
+    @Override
+    protected void printUsage() {
+      System.err.println(SET_CMD_USAGE);
+    }
+
   }  
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/e5d77d09/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/BackupDriver.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/BackupDriver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/BackupDriver.java
index 8eb2ff8..4e668ce 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/BackupDriver.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/BackupDriver.java
@@ -25,7 +25,6 @@ import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.backup.impl.BackupCommands;
-import org.apache.hadoop.hbase.backup.impl.BackupRestoreConstants;
 import org.apache.hadoop.hbase.backup.impl.BackupRestoreConstants.BackupCommand;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.classification.InterfaceStability;
@@ -52,9 +51,9 @@ public class BackupDriver extends AbstractHBaseTool {
     addOptNoArg("debug", "Enable debug loggings");
     addOptNoArg("all", "All tables");
     addOptWithArg("t", "Table name");
-    addOptWithArg("b", "Bandwidth (MB/s)");
-    addOptWithArg("w", "Number of workers");
-    addOptWithArg("n", "History length");
+    addOptWithArg("b", "Bandwidth per worker (M/R task) in MB/s");
+    addOptWithArg("w", "Number of workers (M/R tasks)");
+    addOptWithArg("n", "History of backup length");
     addOptWithArg("set", "Backup set name");
     addOptWithArg("path", "Backup destination root directory path");
     
@@ -67,8 +66,8 @@ public class BackupDriver extends AbstractHBaseTool {
     String cmd = null;
     String[] remainArgs = null;
     if (args == null || args.length == 0) {
-      BackupCommands.createCommand(getConf(),
-        BackupRestoreConstants.BackupCommand.HELP, null).execute();
+      printToolUsage();
+      return -1;
     } else {
       cmd = args[0];
       remainArgs = new String[args.length - 1];
@@ -93,7 +92,8 @@ public class BackupDriver extends AbstractHBaseTool {
     } else if (BackupCommand.SET.name().equalsIgnoreCase(cmd)) {
       type = BackupCommand.SET;
     } else {
-      System.out.println("Unsupported command for backup: " + cmd);
+      System.err.println("Unsupported command for backup: " + cmd);
+      printToolUsage();
       return -1;
     }
 
@@ -110,7 +110,14 @@ public class BackupDriver extends AbstractHBaseTool {
     if( type == BackupCommand.CREATE && conf != null) {
       ((BackupCommands.CreateCommand) command).setConf(conf);
     }   
-    command.execute();
+    try {
+      command.execute();
+    } catch(IOException e) {
+      if(e.getMessage().equals(BackupCommands.IGNORE)){
+        return -1;
+      }
+      throw e;
+    }
     return 0;
   }
 
@@ -134,5 +141,53 @@ public class BackupDriver extends AbstractHBaseTool {
     System.exit(ret);    
   }
   
+  @Override
+  public int run(String[] args) throws IOException {
+    if (conf == null) {
+      LOG.error("Tool configuration is not initialized");
+      throw new NullPointerException("conf");
+    }
+
+    CommandLine cmd;
+    try {
+      // parse the command line arguments
+      cmd = parseArgs(args);
+      cmdLineArgs = args;
+    } catch (Exception e) {
+      System.err.println("Error when parsing command-line arguments: "+e.getMessage());
+      printToolUsage();
+      return EXIT_FAILURE;
+    }
+
+    if (!sanityCheckOptions(cmd)) {
+      printToolUsage();
+      return EXIT_FAILURE;
+    }
 
+    processOptions(cmd);
+
+    int ret = EXIT_FAILURE;
+    try {
+      ret = doWork();
+    } catch (Exception e) {
+      LOG.error("Error running command-line tool", e);
+      return EXIT_FAILURE;
+    }
+    return ret;
+  }
+  
+  protected boolean sanityCheckOptions(CommandLine cmd) {
+    boolean success = true;
+    for (String reqOpt : requiredOptions) {
+      if (!cmd.hasOption(reqOpt)) {
+        System.err.println("Required option -" + reqOpt + " is missing");
+        success = false;
+      }
+    }
+    return success;
+  }
+  
+  protected void printToolUsage() throws IOException {
+    System.err.println(BackupCommands.USAGE); 
+  }
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/e5d77d09/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/RestoreDriver.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/RestoreDriver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/RestoreDriver.java
index 32ef218..d3237f7 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/RestoreDriver.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/RestoreDriver.java
@@ -47,14 +47,13 @@ public class RestoreDriver extends AbstractHBaseTool {
 
   private static final String OPTION_OVERWRITE = "overwrite";
   private static final String OPTION_CHECK = "check";
-  private static final String OPTION_AUTOMATIC = "automatic";
   private static final String OPTION_SET = "set";
   private static final String OPTION_DEBUG = "debug";
 
 
   private static final String USAGE =
       "Usage: hbase restore [-set set_name] <backup_root_path> <backup_id> <tables> [tableMapping] \n"
-          + "       [-overwrite] [-check] [-automatic]\n"
+          + "       [-overwrite] [-check]\n"
           + " backup_root_path  The parent location where the backup images are stored\n"
           + " backup_id         The id identifying the backup image\n"
           + " table(s)          Table(s) from the backup image to be restored.\n"
@@ -65,8 +64,8 @@ public class RestoreDriver extends AbstractHBaseTool {
           + "   -overwrite      With this option, restore overwrites to the existing table "
           + "if there's any in\n"
           + "                   restore target. The existing table must be online before restore.\n"
-          + "   -check          With this option, restore sequence and dependencies are checked\n"
-          + "                   and verified without executing the restore\n"
+          + "   -check          With this option, sequence and dependencies are checked\n"
+          + "                   and verified without executing the restore command\n"
           + "   -set set_name   Backup set to restore, mutually exclusive with table list <tables>.";
 
     
@@ -80,7 +79,6 @@ public class RestoreDriver extends AbstractHBaseTool {
     addOptNoArg(OPTION_OVERWRITE,
         "Overwrite the data if any of the restore target tables exists");
     addOptNoArg(OPTION_CHECK, "Check restore sequence and dependencies");
-    addOptNoArg(OPTION_AUTOMATIC, "Restore all dependencies");
     addOptNoArg(OPTION_DEBUG,  "Enable debug logging");
     addOptWithArg(OPTION_SET, "Backup set name");
 
@@ -88,7 +86,7 @@ public class RestoreDriver extends AbstractHBaseTool {
     LogUtils.disableUselessLoggers(LOG);
   }
 
-  private int parseAndRun(String[] args) {
+  private int parseAndRun(String[] args) throws IOException{
 
     // enable debug logging
     Logger backupClientLogger = Logger.getLogger("org.apache.hadoop.hbase.backup");
@@ -116,8 +114,8 @@ public class RestoreDriver extends AbstractHBaseTool {
     String[] remainArgs = cmd.getArgs();
     if (remainArgs.length < 3 && !cmd.hasOption(OPTION_SET) ||
         (cmd.hasOption(OPTION_SET) && remainArgs.length < 2)) {
-      System.out.println("ERROR: remain args length="+ remainArgs.length);
-      System.out.println(USAGE);
+      System.err.println("ERROR: remain args length="+ remainArgs.length);
+      printToolUsage();
       return -1;
     } 
 
@@ -133,12 +131,14 @@ public class RestoreDriver extends AbstractHBaseTool {
         try{
           tables = getTablesForSet(conn, setName, conf);
         } catch(IOException e){
-          System.out.println("ERROR: "+ e.getMessage()+" for setName="+setName);
+          System.err.println("ERROR: "+ e.getMessage()+" for setName="+setName);
+          printToolUsage();
           return -2;
         }
         if (tables == null) {
-          System.out.println("ERROR: Backup set '" + setName
+          System.err.println("ERROR: Backup set '" + setName
               + "' is either empty or does not exist");
+          printToolUsage();
           return -3;
         }
         tableMapping = (remainArgs.length > 2) ? remainArgs[2] : null;
@@ -151,8 +151,8 @@ public class RestoreDriver extends AbstractHBaseTool {
       TableName[] tTableArray = BackupServerUtil.parseTableNames(tableMapping);
 
       if (sTableArray != null && tTableArray != null && (sTableArray.length != tTableArray.length)){
-        System.out.println("ERROR: table mapping mismatch: " + tables + " : " + tableMapping);
-        System.out.println(USAGE);
+        System.err.println("ERROR: table mapping mismatch: " + tables + " : " + tableMapping);
+        printToolUsage();
         return -4;
       }
 
@@ -193,4 +193,55 @@ public class RestoreDriver extends AbstractHBaseTool {
     int ret = ToolRunner.run(conf, new RestoreDriver(), args);
     System.exit(ret);
   }
+  
+  @Override
+  public int run(String[] args) throws IOException {
+    if (conf == null) {
+      LOG.error("Tool configuration is not initialized");
+      throw new NullPointerException("conf");
+    }
+
+    CommandLine cmd;
+    try {
+      // parse the command line arguments
+      cmd = parseArgs(args);
+      cmdLineArgs = args;
+    } catch (Exception e) {
+      System.err.println("Error when parsing command-line arguments: " + e.getMessage());
+      printToolUsage();
+      return EXIT_FAILURE;
+    }
+
+    if (!sanityCheckOptions(cmd) || cmd.hasOption(SHORT_HELP_OPTION) ||
+        cmd.hasOption(LONG_HELP_OPTION)) {
+      printToolUsage();
+      return EXIT_FAILURE;
+    }
+
+    processOptions(cmd);
+
+    int ret = EXIT_FAILURE;
+    try {
+      ret = doWork();
+    } catch (Exception e) {
+      LOG.error("Error running command-line tool", e);
+      return EXIT_FAILURE;
+    }
+    return ret;
+  }
+  
+  protected boolean sanityCheckOptions(CommandLine cmd) {
+    boolean success = true;
+    for (String reqOpt : requiredOptions) {
+      if (!cmd.hasOption(reqOpt)) {
+        System.err.println("Required option -" + reqOpt + " is missing");
+        success = false;
+      }
+    }
+    return success;
+  }
+  
+  protected void printToolUsage() throws IOException {
+    System.err.println(USAGE);
+  }
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/e5d77d09/hbase-server/src/main/java/org/apache/hadoop/hbase/util/AbstractHBaseTool.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/AbstractHBaseTool.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/AbstractHBaseTool.java
index a876aef..485eef7 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/AbstractHBaseTool.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/AbstractHBaseTool.java
@@ -44,8 +44,8 @@ public abstract class AbstractHBaseTool implements Tool {
   protected static final int EXIT_SUCCESS = 0;
   protected static final int EXIT_FAILURE = 1;
 
-  private static final String SHORT_HELP_OPTION = "h";
-  private static final String LONG_HELP_OPTION = "help";
+  public static final String SHORT_HELP_OPTION = "h";
+  public static final String LONG_HELP_OPTION = "help";
 
   private static final Log LOG = LogFactory.getLog(AbstractHBaseTool.class);
 
@@ -53,7 +53,7 @@ public abstract class AbstractHBaseTool implements Tool {
 
   protected Configuration conf = null;
 
-  private static final Set<String> requiredOptions = new TreeSet<String>();
+  protected static final Set<String> requiredOptions = new TreeSet<String>();
 
   protected String[] cmdLineArgs = null;
 
@@ -82,7 +82,7 @@ public abstract class AbstractHBaseTool implements Tool {
   }
 
   @Override
-  public final int run(String[] args) throws IOException {
+  public int run(String[] args) throws IOException {
     if (conf == null) {
       LOG.error("Tool configuration is not initialized");
       throw new NullPointerException("conf");
@@ -117,7 +117,7 @@ public abstract class AbstractHBaseTool implements Tool {
     return ret;
   }
 
-  private boolean sanityCheckOptions(CommandLine cmd) {
+  protected boolean sanityCheckOptions(CommandLine cmd) {
     boolean success = true;
     for (String reqOpt : requiredOptions) {
       if (!cmd.hasOption(reqOpt)) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/e5d77d09/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java
new file mode 100644
index 0000000..26f9621
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupCommandLineTool.java
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.backup;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.util.ToolRunner;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestBackupCommandLineTool {
+  Configuration conf;
+  @Before
+  public void setUpBefore() throws Exception {
+    conf = HBaseConfiguration.create();
+  }
+
+  @Test
+  public void testBackupDriverDescribeHelp() throws Exception {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    String[] args = new String[]{"describe", "-help" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+
+    String output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup decsribe <backupId>") >= 0);
+
+    baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    args = new String[]{"describe", "-h" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+    
+    output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup decsribe <backupId>") >= 0);        
+  }
+
+  @Test
+  public void testBackupDriverCreateHelp() throws Exception {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    String[] args = new String[]{"create", "-help" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+    
+    String output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup create") >= 0);
+    
+    baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    args = new String[]{"create", "-h" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+    
+    output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup create") >= 0);
+  }
+
+  @Test
+  public void testBackupDriverHistoryHelp () throws Exception {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    String[] args = new String[]{"history", "-help" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+
+    String output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup history") >= 0);
+
+    baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    args = new String[]{"history", "-h" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+
+    output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup history") >= 0);
+  }
+
+  @Test
+  public void testBackupDriverDeleteHelp () throws Exception {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    String[] args = new String[]{"delete", "-help" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+
+    String output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup delete") >= 0);
+
+    baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    args = new String[]{"delete", "-h" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+
+    output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup delete") >= 0);
+  }
+
+  @Test
+  public void testBackupDriverProgressHelp () throws Exception {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    String[] args = new String[]{"progress", "-help" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+    
+    String output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup progress") >= 0);
+    
+    baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    args = new String[]{"progress", "-h" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+    
+    output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup progress") >= 0);
+  }
+
+  @Test
+  public void testBackupDriverSetHelp () throws Exception {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    String[] args = new String[]{"set", "-help" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+
+    String output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup set") >= 0);
+    
+    baos = new ByteArrayOutputStream();
+    System.setErr(new PrintStream(baos));
+    args = new String[]{"set", "-h" }; 
+    // Run backup
+    ToolRunner.run(conf, new BackupDriver(), args);
+    
+    output = baos.toString();
+    System.out.println(baos.toString());
+    assertTrue(output.indexOf("Usage: hbase backup set") >= 0);
+  }
+}

http://git-wip-us.apache.org/repos/asf/hbase/blob/e5d77d09/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestFullBackupSet.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestFullBackupSet.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestFullBackupSet.java
index 85ff4f7..67ec4a2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestFullBackupSet.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestFullBackupSet.java
@@ -91,7 +91,7 @@ public class TestFullBackupSet extends TestBackupBase {
   @Test
   public void testFullBackupSetDoesNotExist() throws Exception {
 
-    LOG.info("TFBSE test full backup, backup set does not exist");    
+    LOG.info("test full backup, backup set does not exist");    
     String name = "name1";    
     String[] args = new String[]{"create", "full", BACKUP_ROOT_DIR, "-set", name }; 
     // Run backup