diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/util/RestoreServerUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/util/RestoreServerUtil.java index 1b05aa9..57b853f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/util/RestoreServerUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/util/RestoreServerUtil.java @@ -282,96 +282,108 @@ public class RestoreServerUtil { FileSystem fileSys = tableBackupPath.getFileSystem(this.conf); - // get table descriptor first - HTableDescriptor tableDescriptor = null; - - Path tableSnapshotPath = getTableSnapshotPath(backupRootPath, tableName, backupId); - - if (fileSys.exists(tableSnapshotPath)) { - // snapshot path exist means the backup path is in HDFS - // check whether snapshot dir already recorded for target table - if (snapshotMap.get(tableName) != null) { - SnapshotDescription desc = - SnapshotDescriptionUtils.readSnapshotInfo(fileSys, tableSnapshotPath); - SnapshotManifest manifest = SnapshotManifest.open(conf, fileSys, tableSnapshotPath, desc); - tableDescriptor = manifest.getTableDescriptor(); - } else { - tableDescriptor = getTableDesc(tableName); - snapshotMap.put(tableName, getTableInfoPath(tableName)); + try (Connection conn = ConnectionFactory.createConnection(conf); + HBaseAdmin hbadmin = (HBaseAdmin) conn.getAdmin();) { + // get table descriptor first + HTableDescriptor tableDescriptor = null; + try { + tableDescriptor = hbadmin.getTableDescriptor(tableName); + } catch (IOException ioe) { + LOG.debug("unable to obtain table descriptor for " + tableName, ioe); } + if (tableDescriptor == null) { - LOG.debug("Found no table descriptor in the snapshot dir, previous schema would be lost"); + Path tableSnapshotPath = getTableSnapshotPath(backupRootPath, tableName, backupId); + if (fileSys.exists(tableSnapshotPath)) { + // snapshot path exist means the backup path is in HDFS + // check whether snapshot dir already recorded for target table + if (snapshotMap.get(tableName) != null) { + SnapshotDescription desc = + SnapshotDescriptionUtils.readSnapshotInfo(fileSys, tableSnapshotPath); + SnapshotManifest manifest = SnapshotManifest.open(conf,fileSys,tableSnapshotPath,desc); + tableDescriptor = manifest.getTableDescriptor(); + LOG.debug("obtained descriptor from " + manifest); + } else { + tableDescriptor = getTableDesc(tableName); + snapshotMap.put(tableName, getTableInfoPath(tableName)); + LOG.debug("obtained descriptor from snapshot for " + tableName); + } + if (tableDescriptor == null) { + LOG.debug("Found no table descriptor in the snapshot dir, previous schema was lost"); + } + } else if (converted) { + // first check if this is a converted backup image + LOG.error("convert will be supported in a future jira"); + } } - } else if (converted) { - // first check if this is a converted backup image - LOG.error("convert will be supported in a future jira"); - } - Path tableArchivePath = getTableArchivePath(tableName); - if (tableArchivePath == null) { - if (tableDescriptor != null) { - // find table descriptor but no archive dir means the table is empty, create table and exit - if(LOG.isDebugEnabled()) { - LOG.debug("find table descriptor but no archive dir for table " + tableName - + ", will only create table"); + Path tableArchivePath = getTableArchivePath(tableName); + if (tableArchivePath == null) { + if (tableDescriptor != null) { + // find table descriptor but no archive dir => the table is empty, create table and exit + if(LOG.isDebugEnabled()) { + LOG.debug("find table descriptor but no archive dir for table " + tableName + + ", will only create table"); + } + tableDescriptor.setName(newTableName); + checkAndCreateTable(hbadmin, tableBackupPath, tableName, newTableName, null, + tableDescriptor, truncateIfExists); + return; + } else { + throw new IllegalStateException("Cannot restore hbase table because directory '" + + " tableArchivePath is null."); } - tableDescriptor.setName(newTableName); - checkAndCreateTable(tableBackupPath, tableName, newTableName, null, - tableDescriptor, truncateIfExists); - return; - } else { - throw new IllegalStateException("Cannot restore hbase table because directory '" - + " tableArchivePath is null."); } - } - if (tableDescriptor == null) { - tableDescriptor = new HTableDescriptor(newTableName); - } else { - tableDescriptor.setName(newTableName); - } + if (tableDescriptor == null) { + LOG.debug("New descriptor for " + newTableName); + tableDescriptor = new HTableDescriptor(newTableName); + } else { + tableDescriptor.setName(newTableName); + } - if (!converted) { - // record all region dirs: - // load all files in dir - try { - ArrayList regionPathList = getRegionList(tableName); - - // should only try to create the table with all region informations, so we could pre-split - // the regions in fine grain - checkAndCreateTable(tableBackupPath, tableName, newTableName, regionPathList, - tableDescriptor, truncateIfExists); - if (tableArchivePath != null) { - // start real restore through bulkload - // if the backup target is on local cluster, special action needed - Path tempTableArchivePath = checkLocalAndBackup(tableArchivePath); - if (tempTableArchivePath.equals(tableArchivePath)) { - if(LOG.isDebugEnabled()) { - LOG.debug("TableArchivePath for bulkload using existPath: " + tableArchivePath); - } - } else { - regionPathList = getRegionList(tempTableArchivePath); // point to the tempDir - if(LOG.isDebugEnabled()) { - LOG.debug("TableArchivePath for bulkload using tempPath: " + tempTableArchivePath); + if (!converted) { + // record all region dirs: + // load all files in dir + try { + ArrayList regionPathList = getRegionList(tableName); + + // should only try to create the table with all region informations, so we could pre-split + // the regions in fine grain + checkAndCreateTable(hbadmin, tableBackupPath, tableName, newTableName, regionPathList, + tableDescriptor, truncateIfExists); + if (tableArchivePath != null) { + // start real restore through bulkload + // if the backup target is on local cluster, special action needed + Path tempTableArchivePath = checkLocalAndBackup(tableArchivePath); + if (tempTableArchivePath.equals(tableArchivePath)) { + if(LOG.isDebugEnabled()) { + LOG.debug("TableArchivePath for bulkload using existPath: " + tableArchivePath); + } + } else { + regionPathList = getRegionList(tempTableArchivePath); // point to the tempDir + if(LOG.isDebugEnabled()) { + LOG.debug("TableArchivePath for bulkload using tempPath: " + tempTableArchivePath); + } } - } - LoadIncrementalHFiles loader = createLoader(tempTableArchivePath, false); - for (Path regionPath : regionPathList) { - String regionName = regionPath.toString(); - if(LOG.isDebugEnabled()) { - LOG.debug("Restoring HFiles from directory " + regionName); + LoadIncrementalHFiles loader = createLoader(tempTableArchivePath, false); + for (Path regionPath : regionPathList) { + String regionName = regionPath.toString(); + if(LOG.isDebugEnabled()) { + LOG.debug("Restoring HFiles from directory " + regionName); + } + String[] args = { regionName, newTableName.getNameAsString()}; + loader.run(args); } - String[] args = { regionName, newTableName.getNameAsString()}; - loader.run(args); } + // we do not recovered edits + } catch (Exception e) { + throw new IllegalStateException("Cannot restore hbase table", e); } - // we do not recovered edits - } catch (Exception e) { - throw new IllegalStateException("Cannot restore hbase table", e); + } else { + LOG.debug("convert will be supported in a future jira"); } - } else { - LOG.debug("convert will be supported in a future jira"); } } @@ -570,15 +582,11 @@ public class RestoreServerUtil { * @param htd table descriptor * @throws IOException exception */ - private void checkAndCreateTable(Path tableBackupPath, TableName tableName, + private void checkAndCreateTable(HBaseAdmin hbadmin, Path tableBackupPath, TableName tableName, TableName targetTableName, ArrayList regionDirList, HTableDescriptor htd, boolean truncateIfExists) throws IOException { - HBaseAdmin hbadmin = null; - Connection conn = null; try { - conn = ConnectionFactory.createConnection(conf); - hbadmin = (HBaseAdmin) conn.getAdmin(); boolean createNew = false; if (hbadmin.tableExists(targetTableName)) { if(truncateIfExists) { @@ -592,7 +600,7 @@ public class RestoreServerUtil { } else { createNew = true; } - if(createNew){ + if (createNew){ LOG.info("Creating target table '" + targetTableName + "'"); // if no region directory given, create the table and return if (regionDirList == null || regionDirList.size() == 0) { @@ -605,13 +613,6 @@ public class RestoreServerUtil { } } catch (Exception e) { throw new IOException(e); - } finally { - if (hbadmin != null) { - hbadmin.close(); - } - if(conn != null){ - conn.close(); - } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupBase.java index da1c6c1..d851240 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestBackupBase.java @@ -68,6 +68,7 @@ public class TestBackupBase { protected static HBaseTestingUtility TEST_UTIL; protected static HBaseTestingUtility TEST_UTIL2; protected static TableName table1 = TableName.valueOf("table1"); + protected static HTableDescriptor table1Desc; protected static TableName table2 = TableName.valueOf("table2"); protected static TableName table3 = TableName.valueOf("table3"); protected static TableName table4 = TableName.valueOf("table4"); @@ -209,6 +210,7 @@ public class TestBackupBase { HColumnDescriptor fam = new HColumnDescriptor(famName); desc.addFamily(fam); ha.createTable(desc); + table1Desc = desc; Connection conn = ConnectionFactory.createConnection(conf1); HTable table = (HTable) conn.getTable(table1); loadTable(table); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupNoDataLoss.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupNoDataLoss.java index c3ad7d4..fc5fa6b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupNoDataLoss.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupNoDataLoss.java @@ -20,10 +20,12 @@ package org.apache.hadoop.hbase.backup; import static org.junit.Assert.assertTrue; +import java.io.IOException; import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.BackupAdmin; import org.apache.hadoop.hbase.client.Connection; @@ -43,6 +45,17 @@ import com.google.common.collect.Lists; public class TestIncrementalBackupNoDataLoss extends TestBackupBase { private static final Log LOG = LogFactory.getLog(TestIncrementalBackupNoDataLoss.class); + HTable insertIntoTable(Connection conn, byte[] family, int numRows) throws IOException { + HTable t1 = (HTable) conn.getTable(table1); + Put p1; + for (int i = 0; i < numRows; i++) { + p1 = new Put(Bytes.toBytes("row-t1" + i)); + p1.addColumn(family, qualName, Bytes.toBytes("val" + i)); + t1.put(p1); + } + return t1; + } + // implement all test cases in 1 test since incremental backup/restore has dependencies @Test public void TestIncBackupRestore() throws Exception { @@ -54,13 +67,7 @@ public class TestIncrementalBackupNoDataLoss extends TestBackupBase { assertTrue(checkSucceeded(backupIdFull)); Connection conn = ConnectionFactory.createConnection(conf1); // #2 - insert some data to table - HTable t1 = (HTable) conn.getTable(table1); - Put p1; - for (int i = 0; i < NB_ROWS_IN_BATCH; i++) { - p1 = new Put(Bytes.toBytes("row-t1" + i)); - p1.addColumn(famName, qualName, Bytes.toBytes("val" + i)); - t1.put(p1); - } + HTable t1 = insertIntoTable(conn, famName, NB_ROWS_IN_BATCH); Assert.assertThat(TEST_UTIL.countRows(t1), CoreMatchers.equalTo(NB_ROWS_IN_BATCH * 2)); t1.close(); @@ -76,8 +83,14 @@ public class TestIncrementalBackupNoDataLoss extends TestBackupBase { Assert.assertThat(TEST_UTIL.countRows(t2), CoreMatchers.equalTo(NB_ROWS_IN_BATCH + 5)); t2.close(); - // #3 - incremental backup for table1 + // add column family to table1 + final byte[] fam2Name = Bytes.toBytes("f2"); + table1Desc.addFamily(new HColumnDescriptor(fam2Name)); + TEST_UTIL.modifyTableSync(TEST_UTIL.getAdmin(), table1Desc); + + HTable t3 = insertIntoTable(conn, fam2Name, 6); + // #3 - incremental backup for table1 tables = Lists.newArrayList(table1); String backupIdInc1 = incrementalTableBackup(tables); assertTrue(checkSucceeded(backupIdInc1));