commit 2ee54a9ccb1da96cc4ef5ee124a4ddf0a2e7b4bd Author: Devaraj Das Date: Fri Sep 5 14:27:48 2014 -0700 "HBASE-11903. Directly invoking split & merge of replica regions should be disallowed (backported patch of 11903-1.txt on jira)" diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java index 6a171ad..0baafb5 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java @@ -147,6 +147,7 @@ import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.util.StringUtils; import org.apache.zookeeper.KeeperException; +import com.google.common.annotations.VisibleForTesting; import com.google.protobuf.ByteString; import com.google.protobuf.ServiceException; @@ -1885,6 +1886,12 @@ public class HBaseAdmin implements Abortable, Closeable { public void mergeRegions(final byte[] encodedNameOfRegionA, final byte[] encodedNameOfRegionB, final boolean forcible) throws IOException { + Pair pair = getRegion(encodedNameOfRegionA, new CatalogTracker(conf)); + if (pair != null && pair.getFirst().getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID) + throw new IllegalArgumentException("Can't invoke merge on non-default regions directly"); + pair = getRegion(encodedNameOfRegionB, new CatalogTracker(conf)); + if (pair != null && pair.getFirst().getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID) + throw new IllegalArgumentException("Can't invoke merge on non-default regions directly"); MasterKeepAliveConnection master = connection .getKeepAliveMasterService(); try { @@ -1956,6 +1963,11 @@ public class HBaseAdmin implements Abortable, Closeable { Pair regionServerPair = getRegion(tableNameOrRegionName, ct); if (regionServerPair != null) { + if (regionServerPair.getFirst() != null && + regionServerPair.getFirst().getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID) { + throw new IllegalArgumentException("Can't split replicas directly. " + + "Replicas are auto-split when their primary is split."); + } if (regionServerPair.getSecond() == null) { throw new NoServerForRegionException(Bytes.toStringBinary(tableNameOrRegionName)); } else { @@ -1974,7 +1986,8 @@ public class HBaseAdmin implements Abortable, Closeable { // check for parents if (r.isSplitParent()) continue; // if a split point given, only split that particular region - if (splitPoint != null && !r.containsRow(splitPoint)) continue; + if (r.getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID || + (splitPoint != null && !r.containsRow(splitPoint))) continue; // call out to region server to do split now split(pair.getSecond(), pair.getFirst(), splitPoint); } @@ -1984,7 +1997,8 @@ public class HBaseAdmin implements Abortable, Closeable { } } - private void split(final ServerName sn, final HRegionInfo hri, + @VisibleForTesting + public void split(final ServerName sn, final HRegionInfo hri, byte[] splitPoint) throws IOException { if (hri.getStartKey() != null && splitPoint != null && Bytes.compareTo(hri.getStartKey(), splitPoint) == 0) { @@ -2057,8 +2071,17 @@ public class HBaseAdmin implements Abortable, Closeable { LOG.warn("No serialized HRegionInfo in " + data); return true; } - if (!encodedName.equals(info.getEncodedName())) return true; - ServerName sn = HRegionInfo.getServerName(data); + RegionLocations rl = MetaReader.getRegionLocations(data); + boolean matched = false; + ServerName sn = null; + for (HRegionLocation h : rl.getRegionLocations()) { + if (h != null && encodedName.equals(h.getRegionInfo().getEncodedName())) { + sn = h.getServerName(); + info = h.getRegionInfo(); + matched = true; + } + } + if (!matched) return true; result.set(new Pair(info, sn)); return false; // found the region, stop } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index f09cd0a..737b1aa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -1662,6 +1662,11 @@ MasterServices, Server { HRegionInfo regionInfoA = regionStateA.getRegion(); HRegionInfo regionInfoB = regionStateB.getRegion(); + if (regionInfoA.getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID || + regionInfoB.getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID) { + throw new ServiceException(new MergeRegionException("Can't merge non-default replicas")); + } + if (regionInfoA.compareTo(regionInfoB) == 0) { throw new ServiceException(new MergeRegionException( "Unable to merge a region to itself " + regionInfoA + ", " + regionInfoB)); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index b7828b1..e5ade81 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -104,6 +104,7 @@ import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.DroppedSnapshotException; import org.apache.hadoop.hbase.exceptions.FailedSanityCheckException; +import org.apache.hadoop.hbase.exceptions.MergeRegionException; import org.apache.hadoop.hbase.exceptions.OperationConflictException; import org.apache.hadoop.hbase.exceptions.OutOfOrderScannerNextException; import org.apache.hadoop.hbase.exceptions.RegionMovedException; @@ -3941,6 +3942,10 @@ public class HRegionServer implements ClientProtos.ClientService.BlockingInterfa checkOpen(); requestCount.increment(); HRegion region = getRegion(request.getRegion()); + if (region.getRegionInfo().getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID) { + throw new IOException("Can't split replicas directly. " + + "Replicas are auto-split when their primary is split."); + } region.startRegionOperation(Operation.SPLIT_REGION); LOG.info("Splitting " + region.getRegionNameAsString()); region.flushcache(); @@ -3973,6 +3978,9 @@ public class HRegionServer implements ClientProtos.ClientService.BlockingInterfa requestCount.increment(); HRegion regionA = getRegion(request.getRegionA()); HRegion regionB = getRegion(request.getRegionB()); + if (regionA.getRegionInfo().getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID || + regionB.getRegionInfo().getReplicaId() != HRegionInfo.DEFAULT_REPLICA_ID) + throw new ServiceException(new MergeRegionException("Can't merge non-default replicas")); boolean forcible = request.getForcible(); regionA.startRegionOperation(Operation.MERGE_REGION); regionB.startRegionOperation(Operation.MERGE_REGION); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java index 41b17ba..e8c527f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java @@ -56,11 +56,16 @@ import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.ZooKeeperConnectionException; import org.apache.hadoop.hbase.catalog.CatalogTracker; +import org.apache.hadoop.hbase.catalog.MetaReader; import org.apache.hadoop.hbase.constraint.ConstraintException; +import org.apache.hadoop.hbase.exceptions.MergeRegionException; import org.apache.hadoop.hbase.executor.EventHandler; import org.apache.hadoop.hbase.master.AssignmentManager; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; +import org.apache.hadoop.hbase.protobuf.RequestConverter; +import org.apache.hadoop.hbase.protobuf.generated.AdminProtos.AdminService; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.DispatchMergingRegionsRequest; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.regionserver.wal.HLogUtilsForTests; @@ -70,6 +75,10 @@ import org.apache.hadoop.hbase.zookeeper.ZKTableReadOnly; import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; import org.junit.*; import org.junit.experimental.categories.Category; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import com.google.protobuf.ServiceException; @@ -1051,6 +1060,126 @@ public class TestAdmin { table.close(); } + @Test + public void testSplitAndMergeWithReplicaTable() throws Exception { + // The test tries to directly split replica regions and directly merge replica regions. These + // are not allowed. The test validates that. Then the test does a valid split/merge of allowed + // regions. + // Set up a table with 3 regions and replication set to 3 + TableName tableName = TableName.valueOf("testSplitAndMergeWithReplicaTable"); + HTableDescriptor desc = new HTableDescriptor(tableName); + desc.setRegionReplication(3); + byte[] cf = "f".getBytes(); + HColumnDescriptor hcd = new HColumnDescriptor(cf); + desc.addFamily(hcd); + byte[][] splitRows = new byte[2][]; + splitRows[0] = new byte[]{(byte)'4'}; + splitRows[1] = new byte[]{(byte)'7'}; + TEST_UTIL.getHBaseAdmin().createTable(desc, splitRows); + List oldRegions; + do { + oldRegions = TEST_UTIL.getHBaseCluster().getRegions(tableName); + Thread.sleep(10); + } while (oldRegions.size() != 9); //3 regions * 3 replicas + // write some data to the table + HTable ht = new HTable(TEST_UTIL.getConfiguration(), tableName); + List puts = new ArrayList(); + byte[] qualifier = "c".getBytes(); + Put put = new Put(new byte[]{(byte)'1'}); + put.add(cf, qualifier, "100".getBytes()); + puts.add(put); + put = new Put(new byte[]{(byte)'6'}); + put.add(cf, qualifier, "100".getBytes()); + puts.add(put); + put = new Put(new byte[]{(byte)'8'}); + put.add(cf, qualifier, "100".getBytes()); + puts.add(put); + ht.put(puts); + ht.flushCommits(); + ht.close(); + List> regions = MetaReader.getTableRegionsAndLocations( + new CatalogTracker(TEST_UTIL.getConfiguration()), tableName); + boolean gotException = false; + // the element at index 1 would be a replica (since the metareader gives us ordered + // regions). Try splitting that region via the split API . Should fail + try { + TEST_UTIL.getHBaseAdmin().split(regions.get(1).getFirst().getRegionName()); + } catch (IllegalArgumentException ex) { + gotException = true; + } + assertTrue(gotException); + gotException = false; + // the element at index 1 would be a replica (since the metareader gives us ordered + // regions). Try splitting that region via a different split API (the difference is + // this API goes direct to the regionserver skipping any checks in the admin). Should fail + try { + TEST_UTIL.getHBaseAdmin().split(regions.get(1).getSecond(), regions.get(1).getFirst(), + new byte[]{(byte)'1'}); + } catch (IOException ex) { + gotException = true; + } + assertTrue(gotException); + gotException = false; + // Try merging a replica with another. Should fail. + try { + TEST_UTIL.getHBaseAdmin().mergeRegions(regions.get(1).getFirst().getEncodedNameAsBytes(), + regions.get(2).getFirst().getEncodedNameAsBytes(), true); + } catch (IllegalArgumentException m) { + gotException = true; + } + assertTrue(gotException); + // Try going to the master directly (that will skip the check in admin) + try { + DispatchMergingRegionsRequest request = RequestConverter + .buildDispatchMergingRegionsRequest(regions.get(1).getFirst().getEncodedNameAsBytes(), + regions.get(2).getFirst().getEncodedNameAsBytes(), true); + TEST_UTIL.getHBaseAdmin().getConnection().getMaster().dispatchMergingRegions(null, request); + } catch (ServiceException m) { + Throwable t = m.getCause(); + do { + if (t instanceof MergeRegionException) { + gotException = true; + break; + } + t = t.getCause(); + } while (t != null); + } + assertTrue(gotException); + gotException = false; + // Try going to the regionservers directly + // first move the region to the same regionserver + if (!regions.get(2).getSecond().equals(regions.get(1).getSecond())) { + moveRegionAndWait(regions.get(2).getFirst(), regions.get(1).getSecond()); + } + try { + AdminService.BlockingInterface admin = TEST_UTIL.getHBaseAdmin().getConnection() + .getAdmin(regions.get(1).getSecond()); + ProtobufUtil.mergeRegions(admin, regions.get(1).getFirst(), regions.get(2).getFirst(), true); + } catch (MergeRegionException mm) { + gotException = true; + } + assertTrue(gotException); + } + + private void moveRegionAndWait(HRegionInfo destRegion, ServerName destServer) + throws InterruptedException, MasterNotRunningException, + ZooKeeperConnectionException, IOException { + HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster(); + TEST_UTIL.getHBaseAdmin().move( + destRegion.getEncodedNameAsBytes(), + Bytes.toBytes(destServer.getServerName())); + while (true) { + ServerName serverName = master.getAssignmentManager() + .getRegionStates().getRegionServerOfRegion(destRegion); + if (serverName != null && serverName.equals(destServer)) { + TEST_UTIL.assertRegionOnServer( + destRegion, serverName, 200); + break; + } + Thread.sleep(10); + } + } + /** * HADOOP-2156 * @throws IOException