hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From te...@apache.org
Subject hbase git commit: HBASE-20008 [backport] NullPointerException when restoring a snapshot after splitting a region
Date Wed, 21 Feb 2018 04:30:04 GMT
Repository: hbase
Updated Branches:
  refs/heads/branch-1.4 451f2fec0 -> 491adbc5f


HBASE-20008 [backport] NullPointerException when restoring a snapshot after splitting a region

Signed-off-by: tedyu <yuzhihong@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/491adbc5
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/491adbc5
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/491adbc5

Branch: refs/heads/branch-1.4
Commit: 491adbc5fa2d0327b88ced2b2f58f06922bfe8a7
Parents: 451f2fe
Author: Toshihiro Suzuki <brfrn169@gmail.com>
Authored: Thu Feb 15 18:03:02 2018 +0900
Committer: tedyu <yuzhihong@gmail.com>
Committed: Tue Feb 20 20:29:56 2018 -0800

----------------------------------------------------------------------
 .../master/snapshot/RestoreSnapshotHandler.java |  5 +-
 .../hbase/snapshot/RestoreSnapshotHelper.java   | 50 ++++++++++--------
 .../client/TestRestoreSnapshotFromClient.java   | 54 ++++++++++++++++++++
 3 files changed, 85 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/491adbc5/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java
index 0ed75a3..f9aea13 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/RestoreSnapshotHandler.java
@@ -183,8 +183,9 @@ public class RestoreSnapshotHandler extends TableEventHandler implements
Snapsho
       String msg = "restore snapshot=" + ClientSnapshotDescriptionUtils.toString(snapshot)
           + " failed. Try re-running the restore command.";
       LOG.error(msg, e);
-      monitor.receive(new ForeignException(masterServices.getServerName().toString(), e));
-      throw new RestoreSnapshotException(msg, e);
+      IOException rse = new RestoreSnapshotException(msg, e);
+      monitor.receive(new ForeignException(masterServices.getServerName().toString(), rse));
+      throw rse;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/491adbc5/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java
----------------------------------------------------------------------
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..fb535de 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<HRegionInfo> regionsToAdd = new ArrayList<HRegionInfo>(regionNames.size());
     if (regionNames.size() > 0) {
-      List<HRegionInfo> regionsToAdd = new ArrayList<HRegionInfo>(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-20008: 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<String, String> daughters = parentsMap.get(clonedRegionName);
       if (daughters == null) {
-        daughters = new Pair<String, String>(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<String, String>(regionName, regionName);
         parentsMap.put(clonedRegionName, daughters);
       } else if (!regionName.equals(daughters.getFirst())) {
         daughters.setSecond(regionName);

http://git-wip-us.apache.org/repos/asf/hbase/blob/491adbc5/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java
----------------------------------------------------------------------
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..6f56609 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
@@ -22,8 +22,11 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.io.InterruptedIOException;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.CategoryBasedTimeout;
@@ -32,6 +35,12 @@ import org.apache.hadoop.hbase.HColumnDescriptor;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.coprocessor.BaseRegionObserver;
+import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.regionserver.InternalScanner;
+import org.apache.hadoop.hbase.regionserver.ScanType;
+import org.apache.hadoop.hbase.regionserver.Store;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.master.MasterFileSystem;
@@ -282,6 +291,51 @@ public class TestRestoreSnapshotFromClient {
     }
   }
 
+  @Test
+  public void testRestoreSnapshotAfterSplittingRegions() throws IOException, InterruptedException
{
+    // HBASE-20008: Add a coprocessor to delay compactions of the daughter regions. To reproduce
+    // the NullPointerException, we need to delay compactions of the daughter regions after
+    // splitting region.
+    HTableDescriptor tableDescriptor = admin.getTableDescriptor(tableName);
+    tableDescriptor.addCoprocessor(DelayCompactionObserver.class.getName());
+    admin.disableTable(tableName);
+    admin.modifyTable(tableName, tableDescriptor);
+    admin.enableTable(tableName);
+
+    List<HRegionInfo> 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);
+  }
+
+  public static class DelayCompactionObserver extends BaseRegionObserver {
+    @Override
+    public InternalScanner preCompact(ObserverContext<RegionCoprocessorEnvironment>
e,
+        final Store store, final InternalScanner scanner, final ScanType scanType)
+        throws IOException {
+
+      try {
+        // Delay 5 seconds.
+        TimeUnit.SECONDS.sleep(5);
+      } catch (InterruptedException ex) {
+        throw new InterruptedIOException(ex.getMessage());
+      }
+
+      return scanner;
+    }
+  }
+
   // ==========================================================================
   //  Helpers
   // ==========================================================================


Mime
View raw message