From 660a23beebfec514b67dda22d3a83f2674c86a20 Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Thu, 15 Feb 2018 18:03:02 +0900 Subject: [PATCH] HBASE-19980 NullPointerException when restoring a snapshot after splitting a region --- .../hbase/snapshot/RestoreSnapshotHelper.java | 50 ++++++++++++---------- .../client/TestRestoreSnapshotFromClient.java | 20 +++++++++ 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java index 75dac43..5a48721 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java @@ -209,38 +209,39 @@ public class RestoreSnapshotHelper { metaChanges.addRegionToRemove(regionInfo); } } - - // Restore regions using the snapshot data - monitor.rethrowException(); - status.setStatus("Restoring table regions..."); - restoreHdfsRegions(exec, regionManifests, metaChanges.getRegionsToRestore()); - status.setStatus("Finished restoring all table regions."); - - // Remove regions from the current table - monitor.rethrowException(); - status.setStatus("Starting to delete excess regions from table"); - removeHdfsRegions(exec, metaChanges.getRegionsToRemove()); - status.setStatus("Finished deleting excess regions from table."); } // Regions to Add: present in the snapshot but not in the current table + List regionsToAdd = new ArrayList(regionNames.size()); if (regionNames.size() > 0) { - List regionsToAdd = new ArrayList(regionNames.size()); - monitor.rethrowException(); for (String regionName: regionNames) { LOG.info("region to add: " + regionName); regionsToAdd.add(HRegionInfo.convert(regionManifests.get(regionName).getRegionInfo())); } - - // Create new regions cloning from the snapshot - monitor.rethrowException(); - status.setStatus("Cloning regions..."); - HRegionInfo[] clonedRegions = cloneHdfsRegions(exec, regionManifests, regionsToAdd); - metaChanges.setNewRegions(clonedRegions); - status.setStatus("Finished cloning regions."); } + // Create new regions cloning from the snapshot + // HBASE-19980: We need to call cloneHdfsRegions() before restoreHdfsRegions() because + // regionsMap is constructed in cloneHdfsRegions() and it can be used in restoreHdfsRegions(). + monitor.rethrowException(); + status.setStatus("Cloning regions..."); + HRegionInfo[] clonedRegions = cloneHdfsRegions(exec, regionManifests, regionsToAdd); + metaChanges.setNewRegions(clonedRegions); + status.setStatus("Finished cloning regions."); + + // Restore regions using the snapshot data + monitor.rethrowException(); + status.setStatus("Restoring table regions..."); + restoreHdfsRegions(exec, regionManifests, metaChanges.getRegionsToRestore()); + status.setStatus("Finished restoring all table regions."); + + // Remove regions from the current table + monitor.rethrowException(); + status.setStatus("Starting to delete excess regions from table"); + removeHdfsRegions(exec, metaChanges.getRegionsToRemove()); + status.setStatus("Finished deleting excess regions from table."); + return metaChanges; } @@ -655,11 +656,16 @@ public class RestoreSnapshotHelper { // Add the daughter region to the map String regionName = Bytes.toString(regionsMap.get(regionInfo.getEncodedNameAsBytes())); + if (regionName == null) { + regionName = regionInfo.getEncodedName(); + } LOG.debug("Restore reference " + regionName + " to " + clonedRegionName); synchronized (parentsMap) { Pair daughters = parentsMap.get(clonedRegionName); if (daughters == null) { - daughters = new Pair(regionName, null); + // In case one side of the split is already compacted, regionName is put as both first and + // second of Pair + daughters = new Pair(regionName, regionName); parentsMap.put(clonedRegionName, daughters); } else if (!regionName.equals(daughters.getFirst())) { daughters.setSecond(regionName); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java index f81cea9..c9d8f42 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java @@ -23,6 +23,7 @@ import static org.junit.Assert.fail; import java.io.IOException; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.apache.hadoop.fs.Path; @@ -282,6 +283,25 @@ public class TestRestoreSnapshotFromClient { } } + @Test + public void testRestoreSnapshotAfterSplittingRegions() throws IOException, InterruptedException { + List regionInfos = admin.getTableRegions(tableName); + RegionReplicaUtil.removeNonDefaultRegions(regionInfos); + + // Split the first region + splitRegion(regionInfos.get(0)); + + // Take a snapshot + admin.snapshot(snapshotName1, tableName); + + // Restore the snapshot + admin.disableTable(tableName); + admin.restoreSnapshot(snapshotName1); + admin.enableTable(tableName); + + SnapshotTestingUtils.verifyRowCount(TEST_UTIL, tableName, snapshot1Rows); + } + // ========================================================================== // Helpers // ========================================================================== -- 2.10.1 (Apple Git-78)