hgromer commented on code in PR #6370:
URL: https://github.com/apache/hbase/pull/6370#discussion_r1923569785


##########
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackup.java:
##########
@@ -100,142 +141,227 @@ public void TestIncBackupRestore() throws Exception {
       .build();
     TEST_UTIL.getAdmin().modifyTable(newTable1Desc);
 
+    Connection conn = TEST_UTIL.getConnection();
+    int NB_ROWS_FAM3 = 6;
+    insertIntoTable(conn, table1, fam3Name, 3, NB_ROWS_FAM3).close();
+    insertIntoTable(conn, table1, mobName, 3, NB_ROWS_FAM3).close();
+    Admin admin = conn.getAdmin();
+    BackupAdminImpl client = new BackupAdminImpl(conn);
+    String backupIdFull = takeFullBackup(tables, client);
+    validateRootPathCanBeOverridden(BACKUP_ROOT_DIR, backupIdFull);
+    assertTrue(checkSucceeded(backupIdFull));
+
+    // #2 - insert some data to table
+    Table t1 = insertIntoTable(conn, table1, famName, 1, ADD_ROWS);
+    LOG.debug("writing " + ADD_ROWS + " rows to " + table1);
+    Assert.assertEquals(HBaseTestingUtil.countRows(t1), NB_ROWS_IN_BATCH + 
ADD_ROWS + NB_ROWS_FAM3);
+    LOG.debug("written " + ADD_ROWS + " rows to " + table1);
+    // additionally, insert rows to MOB cf
+    int NB_ROWS_MOB = 111;
+    insertIntoTable(conn, table1, mobName, 3, NB_ROWS_MOB);
+    LOG.debug("written " + NB_ROWS_MOB + " rows to " + table1 + " to Mob 
enabled CF");
+    t1.close();
+    Assert.assertEquals(HBaseTestingUtil.countRows(t1), NB_ROWS_IN_BATCH + 
ADD_ROWS + NB_ROWS_MOB);
+    Table t2 = conn.getTable(table2);
+    Put p2;
+    for (int i = 0; i < 5; i++) {
+      p2 = new Put(Bytes.toBytes("row-t2" + i));
+      p2.addColumn(famName, qualName, Bytes.toBytes("val" + i));
+      t2.put(p2);
+    }
+    Assert.assertEquals(NB_ROWS_IN_BATCH + 5, HBaseTestingUtil.countRows(t2));
+    t2.close();
+    LOG.debug("written " + 5 + " rows to " + table2);
+    // split table1
+    SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
+    List<HRegion> regions = cluster.getRegions(table1);
+    byte[] name = regions.get(0).getRegionInfo().getRegionName();
+    long startSplitTime = EnvironmentEdgeManager.currentTime();
+    try {
+      admin.splitRegionAsync(name).get();
+    } catch (Exception e) {
+      // although split fail, this may not affect following check in current 
API,
+      // exception will be thrown.
+      LOG.debug("region is not splittable, because " + e);
+    }
+    TEST_UTIL.waitTableAvailable(table1);
+    long endSplitTime = EnvironmentEdgeManager.currentTime();
+    // split finished
+    LOG.debug("split finished in =" + (endSplitTime - startSplitTime));
+
+    // #3 - incremental backup for multiple tables
+    tables = Lists.newArrayList(table1, table2);
+    BackupRequest request = createBackupRequest(BackupType.INCREMENTAL, 
tables, BACKUP_ROOT_DIR);
+    String backupIdIncMultiple = client.backupTables(request);
+    assertTrue(checkSucceeded(backupIdIncMultiple));
+    BackupManifest manifest =
+      HBackupFileSystem.getManifest(conf1, new Path(BACKUP_ROOT_DIR), 
backupIdIncMultiple);
+    assertEquals(Sets.newHashSet(table1, table2), new 
HashSet<>(manifest.getTableList()));
+    validateRootPathCanBeOverridden(BACKUP_ROOT_DIR, backupIdIncMultiple);
+
+    // add column family f2 to table1
+    // drop column family f3
+    final byte[] fam2Name = Bytes.toBytes("f2");
+    newTable1Desc = TableDescriptorBuilder.newBuilder(newTable1Desc)
+      
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(fam2Name)).removeColumnFamily(fam3Name)
+      .build();
+    TEST_UTIL.getAdmin().modifyTable(newTable1Desc);
+
+    // check that an incremental backup fails because the CFs don't match
+    final List<TableName> tablesCopy = tables;
+    IOException ex = assertThrows(IOException.class, () -> client
+      .backupTables(createBackupRequest(BackupType.INCREMENTAL, tablesCopy, 
BACKUP_ROOT_DIR)));
+    checkThrowsCFMismatch(ex, List.of(table1));
+    takeFullBackup(tables, client);
+
+    int NB_ROWS_FAM2 = 7;
+    Table t3 = insertIntoTable(conn, table1, fam2Name, 2, NB_ROWS_FAM2);
+    t3.close();
+
+    // Wait for 5 sec to make sure that old WALs were deleted
+    Thread.sleep(5000);
+
+    // #4 - additional incremental backup for multiple tables
+    request = createBackupRequest(BackupType.INCREMENTAL, tables, 
BACKUP_ROOT_DIR);
+    String backupIdIncMultiple2 = client.backupTables(request);
+    assertTrue(checkSucceeded(backupIdIncMultiple2));
+    validateRootPathCanBeOverridden(BACKUP_ROOT_DIR, backupIdIncMultiple2);
+
+    // #5 - restore full backup for all tables
+    TableName[] tablesRestoreFull = new TableName[] { table1, table2 };
+    TableName[] tablesMapFull = new TableName[] { table1_restore, 
table2_restore };
+
+    LOG.debug("Restoring full " + backupIdFull);
+    client.restore(BackupUtils.createRestoreRequest(BACKUP_ROOT_DIR, 
backupIdFull, false,
+      tablesRestoreFull, tablesMapFull, true));
+
+    // #6.1 - check tables for full restore
+    Admin hAdmin = TEST_UTIL.getAdmin();
+    assertTrue(hAdmin.tableExists(table1_restore));
+    assertTrue(hAdmin.tableExists(table2_restore));
+    hAdmin.close();
+
+    // #6.2 - checking row count of tables for full restore
+    Table hTable = conn.getTable(table1_restore);
+    Assert.assertEquals(HBaseTestingUtil.countRows(hTable), NB_ROWS_IN_BATCH + 
NB_ROWS_FAM3);
+    hTable.close();
+
+    hTable = conn.getTable(table2_restore);
+    Assert.assertEquals(NB_ROWS_IN_BATCH, HBaseTestingUtil.countRows(hTable));
+    hTable.close();
+
+    // #7 - restore incremental backup for multiple tables, with overwrite
+    TableName[] tablesRestoreIncMultiple = new TableName[] { table1, table2 };
+    TableName[] tablesMapIncMultiple = new TableName[] { table1_restore, 
table2_restore };
+    client.restore(BackupUtils.createRestoreRequest(BACKUP_ROOT_DIR, 
backupIdIncMultiple2, false,
+      tablesRestoreIncMultiple, tablesMapIncMultiple, true));
+    hTable = conn.getTable(table1_restore);
+
+    LOG.debug("After incremental restore: " + hTable.getDescriptor());
+    int countFamName = TEST_UTIL.countRows(hTable, famName);
+    LOG.debug("f1 has " + countFamName + " rows");
+    Assert.assertEquals(countFamName, NB_ROWS_IN_BATCH + ADD_ROWS);
+
+    int countFam2Name = TEST_UTIL.countRows(hTable, fam2Name);
+    LOG.debug("f2 has " + countFam2Name + " rows");
+    Assert.assertEquals(countFam2Name, NB_ROWS_FAM2);
+
+    int countMobName = TEST_UTIL.countRows(hTable, mobName);
+    LOG.debug("mob has " + countMobName + " rows");
+    Assert.assertEquals(countMobName, NB_ROWS_MOB);
+    hTable.close();
+
+    hTable = conn.getTable(table2_restore);
+    Assert.assertEquals(NB_ROWS_IN_BATCH + 5, 
HBaseTestingUtil.countRows(hTable));
+    hTable.close();
+    admin.close();
+  }
+
+  @Test
+  public void TestIncBackupRestoreWithOriginalSplits() throws Exception {
+    byte[] fam1 = Bytes.toBytes("f");
+    byte[] mobFam = Bytes.toBytes("mob");
+
+    List<TableName> tables = Lists.newArrayList(table1);
+    TableDescriptor newTable1Desc =
+      
TableDescriptorBuilder.newBuilder(table1Desc).setColumnFamily(ColumnFamilyDescriptorBuilder
+        
.newBuilder(mobFam).setMobEnabled(true).setMobThreshold(5L).build()).build();
+    TEST_UTIL.getAdmin().modifyTable(newTable1Desc);
+
     try (Connection conn = ConnectionFactory.createConnection(conf1)) {
-      int NB_ROWS_FAM3 = 6;
-      insertIntoTable(conn, table1, fam3Name, 3, NB_ROWS_FAM3).close();
-      insertIntoTable(conn, table1, mobName, 3, NB_ROWS_FAM3).close();
-      Admin admin = conn.getAdmin();
-      BackupAdminImpl client = new BackupAdminImpl(conn);
+      BackupAdminImpl backupAdmin = new BackupAdminImpl(conn);
       BackupRequest request = createBackupRequest(BackupType.FULL, tables, 
BACKUP_ROOT_DIR);
-      String backupIdFull = takeFullBackup(tables, client);
-      validateRootPathCanBeOverridden(BACKUP_ROOT_DIR, backupIdFull);
-      assertTrue(checkSucceeded(backupIdFull));
-
-      // #2 - insert some data to table
-      Table t1 = insertIntoTable(conn, table1, famName, 1, ADD_ROWS);
-      LOG.debug("writing " + ADD_ROWS + " rows to " + table1);
-      Assert.assertEquals(HBaseTestingUtil.countRows(t1),
-        NB_ROWS_IN_BATCH + ADD_ROWS + NB_ROWS_FAM3);
-      LOG.debug("written " + ADD_ROWS + " rows to " + table1);
-      // additionally, insert rows to MOB cf
-      int NB_ROWS_MOB = 111;
-      insertIntoTable(conn, table1, mobName, 3, NB_ROWS_MOB);
-      LOG.debug("written " + NB_ROWS_MOB + " rows to " + table1 + " to Mob 
enabled CF");
-      t1.close();
-      Assert.assertEquals(HBaseTestingUtil.countRows(t1),
-        NB_ROWS_IN_BATCH + ADD_ROWS + NB_ROWS_MOB);
-      Table t2 = conn.getTable(table2);
-      Put p2;
-      for (int i = 0; i < 5; i++) {
-        p2 = new Put(Bytes.toBytes("row-t2" + i));
-        p2.addColumn(famName, qualName, Bytes.toBytes("val" + i));
-        t2.put(p2);
-      }
-      Assert.assertEquals(NB_ROWS_IN_BATCH + 5, 
HBaseTestingUtil.countRows(t2));
-      t2.close();
-      LOG.debug("written " + 5 + " rows to " + table2);
-      // split table1
-      SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
-      List<HRegion> regions = cluster.getRegions(table1);
-      byte[] name = regions.get(0).getRegionInfo().getRegionName();
-      long startSplitTime = EnvironmentEdgeManager.currentTime();
-      try {
+      String fullBackupId = backupAdmin.backupTables(request);
+      assertTrue(checkSucceeded(fullBackupId));
+
+      TableName[] fromTables = new TableName[] { table1 };
+      TableName[] toTables = new TableName[] { table1_restore };
+      backupAdmin.restore(BackupUtils.createRestoreRequest(BACKUP_ROOT_DIR, 
fullBackupId, false,
+        fromTables, toTables, true, true));
+
+      Table table = conn.getTable(table1_restore);
+      Assert.assertEquals(NB_ROWS_IN_BATCH, HBaseTestingUtil.countRows(table));
+
+      int ROWS_TO_ADD = 1_000;
+      // different IDs so that rows don't overlap
+      insertIntoTable(conn, table1, fam1, 3, ROWS_TO_ADD);
+      insertIntoTable(conn, table1, mobFam, 4, ROWS_TO_ADD);
+
+      Admin admin = conn.getAdmin();

Review Comment:
   I do close the admin 
[here](https://github.com/apache/hbase/pull/6370/files#diff-21c088f224d8731ed61a0c97a72f528097ca02977cbc3195563fc996b1b66c38R364)



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