Repository: hbase Updated Branches: refs/heads/HBASE-7912 6d1e7079f -> d0d1485f2
HBASE-16655 hbase backup describe with incorrect backup id results in NPE (Vladimir and Ted) Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/d0d1485f Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/d0d1485f Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/d0d1485f Branch: refs/heads/HBASE-7912 Commit: d0d1485f2b5f664489ed6a5b5b73ef8a47fe6715 Parents: 6d1e707 Author: tedyu <yuzhih...@gmail.com> Authored: Mon Sep 19 15:00:12 2016 -0700 Committer: tedyu <yuzhih...@gmail.com> Committed: Mon Sep 19 15:00:12 2016 -0700 ---------------------------------------------------------------------- .../hbase/backup/impl/BackupCommands.java | 14 ++++-- .../hbase/backup/TestBackupCommandLineTool.java | 51 +++++++++++++++++++- .../hadoop/hbase/backup/TestBackupDescribe.java | 21 +++----- 3 files changed, 66 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/d0d1485f/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 1884788..3d40da2 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 @@ -326,13 +326,18 @@ public final class BackupCommands { try (final Connection conn = ConnectionFactory.createConnection(conf); final BackupAdmin admin = conn.getAdmin().getBackupAdmin();) { BackupInfo info = admin.getBackupInfo(backupId); + if (info == null) { + System.err.println("ERROR: " + backupId + " does not exist"); + printUsage(); + throw new IOException(INCORRECT_USAGE); + } System.out.println(info.getShortDescription()); } } @Override protected void printUsage() { - System.err.println(DESCRIBE_CMD_USAGE); + System.err.println(DESCRIBE_CMD_USAGE); } } @@ -349,17 +354,16 @@ public final class BackupCommands { if (cmdline == null || cmdline.getArgs() == null || cmdline.getArgs().length != 2) { - System.out.println("No backup id was specified, " + System.err.println("No backup id was specified, " + "will retrieve the most recent (ongoing) sessions"); } String[] args = cmdline.getArgs(); - if (args.length > 2) { + if (args.length != 2) { System.err.println("ERROR: wrong number of arguments: " + args.length); printUsage(); throw new IOException(INCORRECT_USAGE); } - - + String backupId = args == null ? null : args[1]; Configuration conf = getConf() != null? getConf(): HBaseConfiguration.create(); try(final Connection conn = ConnectionFactory.createConnection(conf); http://git-wip-us.apache.org/repos/asf/hbase/blob/d0d1485f/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 index 8330ecb..31a859d 100644 --- 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 @@ -56,7 +56,16 @@ public class TestBackupCommandLineTool { output = baos.toString(); System.out.println(baos.toString()); - assertTrue(output.indexOf("Usage: hbase backup decsribe <backupId>") >= 0); + assertTrue(output.indexOf("Usage: hbase backup decsribe <backupId>") >= 0); + + baos = new ByteArrayOutputStream(); + System.setErr(new PrintStream(baos)); + args = new String[]{"describe" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup decsribe <backupId>") >= 0); } @Test @@ -78,6 +87,15 @@ public class TestBackupCommandLineTool { 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"}; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup create") >= 0); } @Test @@ -99,6 +117,7 @@ public class TestBackupCommandLineTool { output = baos.toString(); System.out.println(baos.toString()); assertTrue(output.indexOf("Usage: hbase backup history") >= 0); + } @Test @@ -120,10 +139,19 @@ public class TestBackupCommandLineTool { 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" }; + 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 { + public void testBackupDriverProgressHelp() throws Exception { ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setErr(new PrintStream(baos)); String[] args = new String[]{"progress", "-help" }; @@ -141,6 +169,15 @@ public class TestBackupCommandLineTool { 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" }; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup progress") >= 0); } @Test @@ -162,6 +199,16 @@ public class TestBackupCommandLineTool { 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"}; + ToolRunner.run(conf, new BackupDriver(), args); + + output = baos.toString(); + System.out.println(baos.toString()); + assertTrue(output.indexOf("Usage: hbase backup set") >= 0); + } @Test http://git-wip-us.apache.org/repos/asf/hbase/blob/d0d1485f/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java index af377c3..8d8e0d3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java @@ -43,8 +43,7 @@ public class TestBackupDescribe extends TestBackupBase { private static final Log LOG = LogFactory.getLog(TestBackupDescribe.class); /** - * Verify that full backup is created on a single table with data correctly. Verify that describe - * works as expected + * Verify that describe works as expected if incorrect backup Id is supplied * @throws Exception */ @Test @@ -52,16 +51,9 @@ public class TestBackupDescribe extends TestBackupBase { LOG.info("test backup describe on a single table with data"); - List<TableName> tableList = Lists.newArrayList(table1); - String backupId = fullTableBackup(tableList); - - LOG.info("backup complete"); - assertTrue(checkSucceeded(backupId)); - - - BackupInfo info = getBackupAdmin().getBackupInfo(backupId); - assertTrue(info.getState() == BackupState.COMPLETE); - + String[] args = new String[]{"describe", "backup_2" }; + int ret = ToolRunner.run(conf1, new BackupDriver(), args); + assertTrue(ret < 0); } @Test @@ -83,13 +75,16 @@ public class TestBackupDescribe extends TestBackupBase { LOG.info("backup complete"); assertTrue(checkSucceeded(backupId)); + BackupInfo info = getBackupAdmin().getBackupInfo(backupId); + assertTrue(info.getState() == BackupState.COMPLETE); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setOut(new PrintStream(baos)); String[] args = new String[]{"describe", backupId }; // Run backup int ret = ToolRunner.run(conf1, new BackupDriver(), args); - assertTrue(ret == 0); + assertTrue(ret == 0); String response = baos.toString(); assertTrue(response.indexOf(backupId) > 0); assertTrue(response.indexOf("COMPLETE") > 0);