Repository: hbase Updated Branches: refs/heads/HBASE-16961 97bfe34af -> fae5f7e66 (forced update)
HBASE-18068 Fix flaky test TestAsyncSnapshotAdminApi - internalRestoreSnapshot() returns future which completes by just getting proc_id from master. Changed it to wait for the procedure to complete. - Refactor TestAsyncSnapshotAdminApi: Add cleanup() which deletes all tables and snapshots after every test run. Simplifies individual tests. Change-Id: Idc30fb699db32d58fd0f60da220953a430f1d3cc Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/23ea2c36 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/23ea2c36 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/23ea2c36 Branch: refs/heads/HBASE-16961 Commit: 23ea2c36f52283631ce665e04f2af433fc25ae55 Parents: 3aac047 Author: Apekshit Sharma <a...@apache.org> Authored: Thu May 18 14:08:01 2017 -0700 Committer: Apekshit Sharma <a...@apache.org> Committed: Mon May 22 09:20:37 2017 -0700 ---------------------------------------------------------------------- .../hadoop/hbase/client/AsyncHBaseAdmin.java | 62 ++-- .../hbase/client/TestAsyncSnapshotAdminApi.java | 284 +++++++++---------- 2 files changed, 158 insertions(+), 188 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/23ea2c36/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java index 5e64fa3..987080f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java @@ -90,7 +90,6 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.GetOnlineRe import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.GetOnlineRegionResponse; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.SplitRegionRequest; import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.SplitRegionResponse; -import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ProcedureDescription; import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.TableSchema; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AbortProcedureRequest; @@ -1528,7 +1527,8 @@ public class AsyncHBaseAdmin implements AsyncAdmin { return future; } - public CompletableFuture<Void> addReplicationPeer(String peerId, ReplicationPeerConfig peerConfig) { + public CompletableFuture<Void> addReplicationPeer(String peerId, + ReplicationPeerConfig peerConfig) { return this .<Void> newMasterCaller() .action( @@ -1798,11 +1798,11 @@ public class AsyncHBaseAdmin implements AsyncAdmin { future.completeExceptionally(err); } else { // Step.2 Restore snapshot - internalRestoreSnapshot(snapshotName, tableName).whenComplete((ret2, err2) -> { + internalRestoreSnapshot(snapshotName, tableName).whenComplete((void2, err2) -> { if (err2 != null) { // Step.3.a Something went wrong during the restore and try to rollback. internalRestoreSnapshot(failSafeSnapshotSnapshotName, tableName) - .whenComplete((ret3, err3) -> { + .whenComplete((void3, err3) -> { if (err3 != null) { future.completeExceptionally(err3); } else { @@ -1864,13 +1864,8 @@ public class AsyncHBaseAdmin implements AsyncAdmin { future.completeExceptionally(err2); } else if (!exists) { // if table does not exist, then just clone snapshot into new table. - internalRestoreSnapshot(snapshotName, finalTableName).whenComplete((ret, err3) -> { - if (err3 != null) { - future.completeExceptionally(err3); - } else { - future.complete(ret); - } - }); + completeConditionalOnFuture(future, + internalRestoreSnapshot(snapshotName, finalTableName)); } else { isTableDisabled(finalTableName).whenComplete((disabled, err4) -> { if (err4 != null) { @@ -1878,14 +1873,8 @@ public class AsyncHBaseAdmin implements AsyncAdmin { } else if (!disabled) { future.completeExceptionally(new TableNotDisabledException(finalTableName)); } else { - restoreSnapshotWithFailSafe(snapshotName, finalTableName, takeFailSafeSnapshot) - .whenComplete((ret, err5) -> { - if (err5 != null) { - future.completeExceptionally(err5); - } else { - future.complete(ret); - } - }); + completeConditionalOnFuture(future, + restoreSnapshotWithFailSafe(snapshotName, finalTableName, takeFailSafeSnapshot)); } }); } @@ -1894,6 +1883,17 @@ public class AsyncHBaseAdmin implements AsyncAdmin { return future; } + private <T> void completeConditionalOnFuture(CompletableFuture<T> dependentFuture, + CompletableFuture<T> parentFuture) { + parentFuture.whenComplete((res, err) -> { + if (err != null) { + dependentFuture.completeExceptionally(err); + } else { + dependentFuture.complete(res); + } + }); + } + @Override public CompletableFuture<Void> cloneSnapshot(String snapshotName, TableName tableName) { CompletableFuture<Void> future = new CompletableFuture<>(); @@ -1903,13 +1903,7 @@ public class AsyncHBaseAdmin implements AsyncAdmin { } else if (exists) { future.completeExceptionally(new TableExistsException(tableName)); } else { - internalRestoreSnapshot(snapshotName, tableName).whenComplete((ret, err2) -> { - if (err2 != null) { - future.completeExceptionally(err2); - } else { - future.complete(ret); - } - }); + completeConditionalOnFuture(future, internalRestoreSnapshot(snapshotName, tableName)); } }); return future; @@ -1924,13 +1918,15 @@ public class AsyncHBaseAdmin implements AsyncAdmin { } catch (IllegalArgumentException e) { return failedFuture(e); } - return this.<Void> newMasterCaller() - .action((controller, stub) -> this - .<RestoreSnapshotRequest, RestoreSnapshotResponse, Void> call(controller, stub, - RestoreSnapshotRequest.newBuilder().setSnapshot(snapshot) - .setNonceGroup(ng.getNonceGroup()).setNonce(ng.newNonce()).build(), - (s, c, req, done) -> s.restoreSnapshot(c, req, done), resp -> null)) - .call(); + return waitProcedureResult( + this.<Long> newMasterCaller() + .action((controller, stub) -> this + .<RestoreSnapshotRequest, RestoreSnapshotResponse, Long> call(controller, stub, + RestoreSnapshotRequest.newBuilder().setSnapshot(snapshot) + .setNonceGroup(ng.getNonceGroup()).setNonce(ng.newNonce()).build(), + (s, c, req, done) -> s.restoreSnapshot(c, req, done), + (resp) -> resp.getProcId())) + .call()); } @Override http://git-wip-us.apache.org/repos/asf/hbase/blob/23ea2c36/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncSnapshotAdminApi.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncSnapshotAdminApi.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncSnapshotAdminApi.java index 108fc7a..9256ec1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncSnapshotAdminApi.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncSnapshotAdminApi.java @@ -22,9 +22,13 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.Bytes; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; import java.io.IOException; import java.util.Collections; @@ -34,82 +38,86 @@ import java.util.regex.Pattern; @Category({ MediumTests.class, ClientTests.class }) public class TestAsyncSnapshotAdminApi extends TestAsyncAdminBase { + String snapshotName1 = "snapshotName1"; + String snapshotName2 = "snapshotName2"; + String snapshotName3 = "snapshotName3"; + + @Rule + public TestName testName = new TestName(); + TableName tableName; + + @Before + public void setup() { + tableName = TableName.valueOf(testName.getMethodName()); + } + + @After + public void cleanup() throws Exception { + admin.deleteSnapshots(".*").get(); + admin.disableTables(".*").get(); + admin.deleteTables(".*").get(); + } + @Test public void testTakeSnapshot() throws Exception { - String snapshotName1 = "snapshotName1"; - String snapshotName2 = "snapshotName2"; - TableName tableName = TableName.valueOf("testTakeSnapshot"); Admin syncAdmin = TEST_UTIL.getAdmin(); - try { - Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); - for (int i = 0; i < 3000; i++) { - table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), - Bytes.toBytes(i))); - } - - admin.snapshot(snapshotName1, tableName).get(); - admin.snapshot(snapshotName2, tableName).get(); - List<SnapshotDescription> snapshots = syncAdmin.listSnapshots(); - Collections.sort(snapshots, (snap1, snap2) -> { - Assert.assertNotNull(snap1); - Assert.assertNotNull(snap1.getName()); - Assert.assertNotNull(snap2); - Assert.assertNotNull(snap2.getName()); - return snap1.getName().compareTo(snap2.getName()); - }); - - Assert.assertEquals(snapshotName1, snapshots.get(0).getName()); - Assert.assertEquals(tableName, snapshots.get(0).getTableName()); - Assert.assertEquals(SnapshotType.FLUSH, snapshots.get(0).getType()); - Assert.assertEquals(snapshotName2, snapshots.get(1).getName()); - Assert.assertEquals(tableName, snapshots.get(1).getTableName()); - Assert.assertEquals(SnapshotType.FLUSH, snapshots.get(1).getType()); - } finally { - syncAdmin.deleteSnapshot(snapshotName1); - syncAdmin.deleteSnapshot(snapshotName2); - TEST_UTIL.deleteTable(tableName); + Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); + for (int i = 0; i < 3000; i++) { + table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), + Bytes.toBytes(i))); } + + admin.snapshot(snapshotName1, tableName).get(); + admin.snapshot(snapshotName2, tableName).get(); + List<SnapshotDescription> snapshots = syncAdmin.listSnapshots(); + Collections.sort(snapshots, (snap1, snap2) -> { + Assert.assertNotNull(snap1); + Assert.assertNotNull(snap1.getName()); + Assert.assertNotNull(snap2); + Assert.assertNotNull(snap2.getName()); + return snap1.getName().compareTo(snap2.getName()); + }); + + Assert.assertEquals(snapshotName1, snapshots.get(0).getName()); + Assert.assertEquals(tableName, snapshots.get(0).getTableName()); + Assert.assertEquals(SnapshotType.FLUSH, snapshots.get(0).getType()); + Assert.assertEquals(snapshotName2, snapshots.get(1).getName()); + Assert.assertEquals(tableName, snapshots.get(1).getTableName()); + Assert.assertEquals(SnapshotType.FLUSH, snapshots.get(1).getType()); } @Test public void testCloneSnapshot() throws Exception { - String snapshotName1 = "snapshotName1"; - TableName tableName = TableName.valueOf("testCloneSnapshot"); TableName tableName2 = TableName.valueOf("testCloneSnapshot2"); Admin syncAdmin = TEST_UTIL.getAdmin(); - try { - Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); - for (int i = 0; i < 3000; i++) { - table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), - Bytes.toBytes(i))); - } + Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); + for (int i = 0; i < 3000; i++) { + table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), + Bytes.toBytes(i))); + } - admin.snapshot(snapshotName1, tableName).get(); - List<SnapshotDescription> snapshots = syncAdmin.listSnapshots(); - Assert.assertEquals(snapshots.size(), 1); - Assert.assertEquals(snapshotName1, snapshots.get(0).getName()); - Assert.assertEquals(tableName, snapshots.get(0).getTableName()); - Assert.assertEquals(SnapshotType.FLUSH, snapshots.get(0).getType()); - - // cloneSnapshot into a existed table. - boolean failed = false; - try { - admin.cloneSnapshot(snapshotName1, tableName).get(); - } catch (Exception e) { - failed = true; - } - Assert.assertTrue(failed); - - // cloneSnapshot into a new table. - Assert.assertTrue(!syncAdmin.tableExists(tableName2)); - admin.cloneSnapshot(snapshotName1, tableName2).get(); - syncAdmin.tableExists(tableName2); - } finally { - syncAdmin.deleteSnapshot(snapshotName1); - TEST_UTIL.deleteTable(tableName); + admin.snapshot(snapshotName1, tableName).get(); + List<SnapshotDescription> snapshots = syncAdmin.listSnapshots(); + Assert.assertEquals(snapshots.size(), 1); + Assert.assertEquals(snapshotName1, snapshots.get(0).getName()); + Assert.assertEquals(tableName, snapshots.get(0).getTableName()); + Assert.assertEquals(SnapshotType.FLUSH, snapshots.get(0).getType()); + + // cloneSnapshot into a existed table. + boolean failed = false; + try { + admin.cloneSnapshot(snapshotName1, tableName).get(); + } catch (Exception e) { + failed = true; } + Assert.assertTrue(failed); + + // cloneSnapshot into a new table. + Assert.assertTrue(!syncAdmin.tableExists(tableName2)); + admin.cloneSnapshot(snapshotName1, tableName2).get(); + syncAdmin.tableExists(tableName2); } private void assertResult(TableName tableName, int expectedRowCount) throws IOException { @@ -131,115 +139,81 @@ public class TestAsyncSnapshotAdminApi extends TestAsyncAdminBase { @Test public void testRestoreSnapshot() throws Exception { - String snapshotName1 = "snapshotName1"; - String snapshotName2 = "snapshotName2"; - TableName tableName = TableName.valueOf("testRestoreSnapshot"); - Admin syncAdmin = TEST_UTIL.getAdmin(); - - try { - Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); - for (int i = 0; i < 3000; i++) { - table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), - Bytes.toBytes(i))); - } - Assert.assertEquals(admin.listSnapshots().get().size(), 0); - - admin.snapshot(snapshotName1, tableName).get(); - admin.snapshot(snapshotName2, tableName).get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 2); - - admin.disableTable(tableName).get(); - admin.restoreSnapshot(snapshotName1, true).get(); - admin.enableTable(tableName).get(); - assertResult(tableName, 3000); - - admin.disableTable(tableName).get(); - admin.restoreSnapshot(snapshotName2, false).get(); - admin.enableTable(tableName).get(); - assertResult(tableName, 3000); - } finally { - syncAdmin.deleteSnapshot(snapshotName1); - syncAdmin.deleteSnapshot(snapshotName2); - TEST_UTIL.deleteTable(tableName); + Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); + for (int i = 0; i < 3000; i++) { + table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), + Bytes.toBytes(i))); } + Assert.assertEquals(admin.listSnapshots().get().size(), 0); + + admin.snapshot(snapshotName1, tableName).get(); + admin.snapshot(snapshotName2, tableName).get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 2); + + admin.disableTable(tableName).get(); + admin.restoreSnapshot(snapshotName1, true).get(); + admin.enableTable(tableName).get(); + assertResult(tableName, 3000); + + admin.disableTable(tableName).get(); + admin.restoreSnapshot(snapshotName2, false).get(); + admin.enableTable(tableName).get(); + assertResult(tableName, 3000); } @Test public void testListSnapshots() throws Exception { - String snapshotName1 = "snapshotName1"; - String snapshotName2 = "snapshotName2"; - String snapshotName3 = "snapshotName3"; - TableName tableName = TableName.valueOf("testListSnapshots"); - Admin syncAdmin = TEST_UTIL.getAdmin(); - - try { - Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); - for (int i = 0; i < 3000; i++) { - table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), - Bytes.toBytes(i))); - } - Assert.assertEquals(admin.listSnapshots().get().size(), 0); - - admin.snapshot(snapshotName1, tableName).get(); - admin.snapshot(snapshotName2, tableName).get(); - admin.snapshot(snapshotName3, tableName).get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 3); - - Assert.assertEquals(admin.listSnapshots("(.*)").get().size(), 3); - Assert.assertEquals(admin.listSnapshots("snapshotName(\\d+)").get().size(), 3); - Assert.assertEquals(admin.listSnapshots("snapshotName[1|3]").get().size(), 2); - Assert.assertEquals(admin.listSnapshots(Pattern.compile("snapshot(.*)")).get().size(), 3); - Assert.assertEquals(admin.listTableSnapshots("testListSnapshots", "s(.*)").get().size(), 3); - Assert.assertEquals(admin.listTableSnapshots("fakeTableName", "snap(.*)").get().size(), 0); - Assert.assertEquals(admin.listTableSnapshots("test(.*)", "snap(.*)[1|3]").get().size(), 2); - - } finally { - syncAdmin.deleteSnapshot(snapshotName1); - syncAdmin.deleteSnapshot(snapshotName2); - syncAdmin.deleteSnapshot(snapshotName3); - TEST_UTIL.deleteTable(tableName); + Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); + for (int i = 0; i < 3000; i++) { + table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), + Bytes.toBytes(i))); } + Assert.assertEquals(admin.listSnapshots().get().size(), 0); + + admin.snapshot(snapshotName1, tableName).get(); + admin.snapshot(snapshotName2, tableName).get(); + admin.snapshot(snapshotName3, tableName).get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 3); + + Assert.assertEquals(admin.listSnapshots("(.*)").get().size(), 3); + Assert.assertEquals(admin.listSnapshots("snapshotName(\\d+)").get().size(), 3); + Assert.assertEquals(admin.listSnapshots("snapshotName[1|3]").get().size(), 2); + Assert.assertEquals(admin.listSnapshots(Pattern.compile("snapshot(.*)")).get().size(), 3); + Assert.assertEquals(admin.listTableSnapshots("testListSnapshots", "s(.*)").get().size(), 3); + Assert.assertEquals(admin.listTableSnapshots("fakeTableName", "snap(.*)").get().size(), 0); + Assert.assertEquals(admin.listTableSnapshots("test(.*)", "snap(.*)[1|3]").get().size(), 2); } @Test public void testDeleteSnapshots() throws Exception { - String snapshotName1 = "snapshotName1"; - String snapshotName2 = "snapshotName2"; - String snapshotName3 = "snapshotName3"; - TableName tableName = TableName.valueOf("testDeleteSnapshots"); - - try { - Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); - for (int i = 0; i < 3000; i++) { - table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), - Bytes.toBytes(i))); - } - Assert.assertEquals(admin.listSnapshots().get().size(), 0); + Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f1")); + for (int i = 0; i < 3000; i++) { + table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f1"), Bytes.toBytes("cq"), + Bytes.toBytes(i))); + } + Assert.assertEquals(admin.listSnapshots().get().size(), 0); - admin.snapshot(snapshotName1, tableName).get(); - admin.snapshot(snapshotName2, tableName).get(); - admin.snapshot(snapshotName3, tableName).get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 3); + admin.snapshot(snapshotName1, tableName).get(); + admin.snapshot(snapshotName2, tableName).get(); + admin.snapshot(snapshotName3, tableName).get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 3); - admin.deleteSnapshot(snapshotName1).get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 2); + admin.deleteSnapshot(snapshotName1).get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 2); - admin.deleteSnapshots("(.*)abc").get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 2); + admin.deleteSnapshots("(.*)abc").get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 2); - admin.deleteSnapshots("(.*)1").get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 2); + admin.deleteSnapshots("(.*)1").get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 2); - admin.deleteTableSnapshots("(.*)", "(.*)1").get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 2); + admin.deleteTableSnapshots("(.*)", "(.*)1").get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 2); - admin.deleteTableSnapshots("(.*)", "(.*)2").get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 1); + admin.deleteTableSnapshots("(.*)", "(.*)2").get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 1); - admin.deleteTableSnapshots("(.*)", "(.*)3").get(); - Assert.assertEquals(admin.listSnapshots().get().size(), 0); - } finally { - TEST_UTIL.deleteTable(tableName); - } + admin.deleteTableSnapshots("(.*)", "(.*)3").get(); + Assert.assertEquals(admin.listSnapshots().get().size(), 0); } } \ No newline at end of file
