diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java index f28ef94..d601491 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterAndRegionObserver.java @@ -54,6 +54,16 @@ public abstract class BaseMasterAndRegionObserver extends BaseRegionObserver } @Override + public void preDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException { + } + + @Override + public void postDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException { + } + + @Override public void preCreateTableHandler( final ObserverContext ctx, HTableDescriptor desc, HRegionInfo[] regions) throws IOException { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java index d005389..0cdf0ad 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java @@ -65,6 +65,16 @@ public class BaseMasterObserver implements MasterObserver { } @Override + public void preDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException { + } + + @Override + public void postDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException { + } + + @Override public void preDeleteTable(ObserverContext ctx, TableName tableName) throws IOException { } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index ede8cd4..51b2df2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -1217,4 +1217,25 @@ public interface MasterObserver extends Coprocessor { */ void postSetNamespaceQuota(final ObserverContext ctx, final String namespace, final Quotas quotas) throws IOException; + + /** + * Called before dispatching region merge request. + * @throws IOException if an error occurred on the coprocessor + * @param ctx + * @param regionA + * @param regionB + * @throws IOException + */ + public void preDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException; + + /** + * called after dispatching the region merge request. + * @param c + * @param regionA + * @param regionB + * @throws IOException + */ + void postDispatchMerge(final ObserverContext c, + final HRegionInfo regionA, final HRegionInfo regionB) throws IOException; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 5fa92c6..5888b3e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -729,6 +729,28 @@ public class MasterCoprocessorHost }); } + public void preDispatchMerge(final HRegionInfo regionInfoA, final HRegionInfo regionInfoB) + throws IOException { + execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { + @Override + public void call(MasterObserver oserver, ObserverContext ctx) + throws IOException { + oserver.preDispatchMerge(ctx, regionInfoA, regionInfoB); + } + }); + } + + public void postDispatchMerge(final HRegionInfo regionInfoA, final HRegionInfo regionInfoB) + throws IOException { + execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { + @Override + public void call(MasterObserver oserver, ObserverContext ctx) + throws IOException { + oserver.postDispatchMerge(ctx, regionInfoA, regionInfoB); + } + }); + } + public boolean preBalance() throws IOException { return execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index 141fa88..4e7fe56 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -19,7 +19,9 @@ package org.apache.hadoop.hbase.master; import java.io.IOException; +import java.io.InterruptedIOException; import java.net.InetAddress; +import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -47,6 +49,7 @@ import org.apache.hadoop.hbase.exceptions.MergeRegionException; import org.apache.hadoop.hbase.exceptions.UnknownProtocolException; import org.apache.hadoop.hbase.ipc.PriorityFunction; import org.apache.hadoop.hbase.ipc.QosPriority; +import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.ipc.RpcServer.BlockingServiceAndInterface; import org.apache.hadoop.hbase.ipc.ServerRpcController; import org.apache.hadoop.hbase.mob.MobUtils; @@ -514,8 +517,8 @@ public class MasterRpcServices extends RSRpcServices "Unable to merge regions not online " + regionStateA + ", " + regionStateB)); } - HRegionInfo regionInfoA = regionStateA.getRegion(); - HRegionInfo regionInfoB = regionStateB.getRegion(); + final HRegionInfo regionInfoA = regionStateA.getRegion(); + final 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")); @@ -524,6 +527,28 @@ public class MasterRpcServices extends RSRpcServices throw new ServiceException(new MergeRegionException( "Unable to merge a region to itself " + regionInfoA + ", " + regionInfoB)); } + try { + if (User.isHBaseSecurityEnabled(master.getConfiguration())) { + User user = RpcServer.getRequestUser(); + try { + user.getUGI().doAs(new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + master.cpHost.preDispatchMerge(regionInfoA, regionInfoB); + return null; + } + }); + } catch (InterruptedException ie) { + InterruptedIOException iioe = new InterruptedIOException(); + iioe.initCause(ie); + throw new ServiceException(iioe); + } + } else { + master.cpHost.preDispatchMerge(regionInfoA, regionInfoB); + } + } catch (IOException ioe) { + throw new ServiceException(ioe); + } if (!forcible && !HRegionInfo.areAdjacent(regionInfoA, regionInfoB)) { throw new ServiceException(new MergeRegionException( @@ -535,6 +560,7 @@ public class MasterRpcServices extends RSRpcServices try { master.dispatchMergingRegions(regionInfoA, regionInfoB, forcible); + master.cpHost.postDispatchMerge(regionInfoA, regionInfoB); } catch (IOException ioe) { throw new ServiceException(ioe); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index bb348a3..43f9833 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -2530,6 +2530,13 @@ public class AccessController extends BaseMasterAndRegionObserver } @Override + public void preDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException { + requirePermission("mergeRegions", regionA.getTable(), null, null, + Action.ADMIN); + } + + @Override public void preMerge(ObserverContext ctx, Region regionA, Region regionB) throws IOException { requirePermission("mergeRegions", regionA.getTableDesc().getTableName(), null, null, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java index 638811a..a7397df 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java @@ -25,6 +25,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -44,8 +45,12 @@ import org.apache.hadoop.hbase.ProcedureInfo; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.ConnectionFactory; +import org.apache.hadoop.hbase.client.HTable; import org.apache.hadoop.hbase.client.RegionLocator; import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TestFromClientSide; import org.apache.hadoop.hbase.master.AssignmentManager; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.MasterCoprocessorHost; @@ -167,6 +172,8 @@ public class TestMasterObserver { private boolean postGetTableDescriptorsCalled; private boolean postGetTableNamesCalled; private boolean preGetTableNamesCalled; + private boolean preDispatchMergeCalled; + private boolean postDispatchMergeCalled; public void enableBypass(boolean bypass) { this.bypass = bypass; @@ -249,6 +256,24 @@ public class TestMasterObserver { postGetTableDescriptorsCalled = false; postGetTableNamesCalled = false; preGetTableNamesCalled = false; + preDispatchMergeCalled = false; + postDispatchMergeCalled = false; + } + + @Override + public void preDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException { + preDispatchMergeCalled = true; + } + + @Override + public void postDispatchMerge(final ObserverContext ctx, + HRegionInfo regionA, HRegionInfo regionB) throws IOException { + postDispatchMergeCalled = true; + } + + public boolean wasDispatchMergeCalled() { + return preDispatchMergeCalled && postDispatchMergeCalled; } @Override @@ -1347,155 +1372,166 @@ public class TestMasterObserver { // create a table HTableDescriptor htd = new HTableDescriptor(tableName); htd.addFamily(new HColumnDescriptor(TEST_FAMILY)); - Admin admin = UTIL.getHBaseAdmin(); - - tableCreationLatch = new CountDownLatch(1); - admin.createTable(htd); - // preCreateTable can't bypass default action. - assertTrue("Test table should be created", cp.wasCreateTableCalled()); - tableCreationLatch.await(); - assertTrue("Table pre create handler called.", cp + try(Connection connection = ConnectionFactory.createConnection(UTIL.getConfiguration()); + Admin admin = connection.getAdmin()) { + tableCreationLatch = new CountDownLatch(1); + admin.createTable(htd, Arrays.copyOfRange(HBaseTestingUtility.KEYS, + 1, HBaseTestingUtility.KEYS.length)); + + // preCreateTable can't bypass default action. + assertTrue("Test table should be created", cp.wasCreateTableCalled()); + tableCreationLatch.await(); + assertTrue("Table pre create handler called.", cp .wasPreCreateTableHandlerCalled()); - assertTrue("Table create handler should be called.", + assertTrue("Table create handler should be called.", cp.wasCreateTableHandlerCalled()); - tableCreationLatch = new CountDownLatch(1); - admin.disableTable(tableName); - assertTrue(admin.isTableDisabled(tableName)); - // preDisableTable can't bypass default action. - assertTrue("Coprocessor should have been called on table disable", - cp.wasDisableTableCalled()); - assertTrue("Disable table handler should be called.", + RegionLocator regionLocator = connection.getRegionLocator(htd.getTableName()); + List regions = regionLocator.getAllRegionLocations(); + + admin.mergeRegions(regions.get(0).getRegionInfo().getEncodedNameAsBytes(), + regions.get(1).getRegionInfo().getEncodedNameAsBytes(), true); + assertTrue("Coprocessor should have been called on region merge", + cp.wasDispatchMergeCalled()); + + tableCreationLatch = new CountDownLatch(1); + admin.disableTable(tableName); + assertTrue(admin.isTableDisabled(tableName)); + // preDisableTable can't bypass default action. + assertTrue("Coprocessor should have been called on table disable", + cp.wasDisableTableCalled()); + assertTrue("Disable table handler should be called.", cp.wasDisableTableHandlerCalled()); - // enable - assertFalse(cp.wasEnableTableCalled()); - admin.enableTable(tableName); - assertTrue(admin.isTableEnabled(tableName)); - // preEnableTable can't bypass default action. - assertTrue("Coprocessor should have been called on table enable", - cp.wasEnableTableCalled()); - assertTrue("Enable table handler should be called.", + // enable + assertFalse(cp.wasEnableTableCalled()); + admin.enableTable(tableName); + assertTrue(admin.isTableEnabled(tableName)); + // preEnableTable can't bypass default action. + assertTrue("Coprocessor should have been called on table enable", + cp.wasEnableTableCalled()); + assertTrue("Enable table handler should be called.", cp.wasEnableTableHandlerCalled()); - admin.disableTable(tableName); - assertTrue(admin.isTableDisabled(tableName)); + admin.disableTable(tableName); + assertTrue(admin.isTableDisabled(tableName)); - // modify table - htd.setMaxFileSize(512 * 1024 * 1024); - modifyTableSync(admin, tableName, htd); - // preModifyTable can't bypass default action. - assertTrue("Test table should have been modified", - cp.wasModifyTableCalled()); + // modify table + htd.setMaxFileSize(512 * 1024 * 1024); + modifyTableSync(admin, tableName, htd); + // preModifyTable can't bypass default action. + assertTrue("Test table should have been modified", + cp.wasModifyTableCalled()); - // add a column family - admin.addColumnFamily(tableName, new HColumnDescriptor(TEST_FAMILY2)); - assertTrue("New column family shouldn't have been added to test table", - cp.preAddColumnCalledOnly()); + // add a column family + admin.addColumnFamily(tableName, new HColumnDescriptor(TEST_FAMILY2)); + assertTrue("New column family shouldn't have been added to test table", + cp.preAddColumnCalledOnly()); - // modify a column family - HColumnDescriptor hcd1 = new HColumnDescriptor(TEST_FAMILY2); - hcd1.setMaxVersions(25); - admin.modifyColumnFamily(tableName, hcd1); - assertTrue("Second column family should be modified", - cp.preModifyColumnCalledOnly()); + // modify a column family + HColumnDescriptor hcd1 = new HColumnDescriptor(TEST_FAMILY2); + hcd1.setMaxVersions(25); + admin.modifyColumnFamily(tableName, hcd1); + assertTrue("Second column family should be modified", + cp.preModifyColumnCalledOnly()); - // truncate table - admin.truncateTable(tableName, false); + // truncate table + admin.truncateTable(tableName, false); - // delete table - admin.disableTable(tableName); - assertTrue(admin.isTableDisabled(tableName)); - deleteTable(admin, tableName); - assertFalse("Test table should have been deleted", + // delete table + admin.disableTable(tableName); + assertTrue(admin.isTableDisabled(tableName)); + deleteTable(admin, tableName); + assertFalse("Test table should have been deleted", admin.tableExists(tableName)); - // preDeleteTable can't bypass default action. - assertTrue("Coprocessor should have been called on table delete", + // preDeleteTable can't bypass default action. + assertTrue("Coprocessor should have been called on table delete", cp.wasDeleteTableCalled()); - assertTrue("Delete table handler should be called.", + assertTrue("Delete table handler should be called.", cp.wasDeleteTableHandlerCalled()); - // turn off bypass, run the tests again - cp.enableBypass(false); - cp.resetStates(); + // turn off bypass, run the tests again + cp.enableBypass(false); + cp.resetStates(); - admin.createTable(htd); - assertTrue("Test table should be created", cp.wasCreateTableCalled()); - tableCreationLatch.await(); - assertTrue("Table pre create handler called.", cp + admin.createTable(htd); + assertTrue("Test table should be created", cp.wasCreateTableCalled()); + tableCreationLatch.await(); + assertTrue("Table pre create handler called.", cp .wasPreCreateTableHandlerCalled()); - assertTrue("Table create handler should be called.", + assertTrue("Table create handler should be called.", cp.wasCreateTableHandlerCalled()); - // disable - assertFalse(cp.wasDisableTableCalled()); - assertFalse(cp.wasDisableTableHandlerCalled()); - admin.disableTable(tableName); - assertTrue(admin.isTableDisabled(tableName)); - assertTrue("Coprocessor should have been called on table disable", - cp.wasDisableTableCalled()); - assertTrue("Disable table handler should be called.", + // disable + assertFalse(cp.wasDisableTableCalled()); + assertFalse(cp.wasDisableTableHandlerCalled()); + admin.disableTable(tableName); + assertTrue(admin.isTableDisabled(tableName)); + assertTrue("Coprocessor should have been called on table disable", + cp.wasDisableTableCalled()); + assertTrue("Disable table handler should be called.", cp.wasDisableTableHandlerCalled()); - // modify table - htd.setMaxFileSize(512 * 1024 * 1024); - modifyTableSync(admin, tableName, htd); - assertTrue("Test table should have been modified", + // modify table + htd.setMaxFileSize(512 * 1024 * 1024); + modifyTableSync(admin, tableName, htd); + assertTrue("Test table should have been modified", cp.wasModifyTableCalled()); - // add a column family - admin.addColumnFamily(tableName, new HColumnDescriptor(TEST_FAMILY2)); - assertTrue("New column family should have been added to test table", + // add a column family + admin.addColumnFamily(tableName, new HColumnDescriptor(TEST_FAMILY2)); + assertTrue("New column family should have been added to test table", cp.wasAddColumnCalled()); - assertTrue("Add column handler should be called.", + assertTrue("Add column handler should be called.", cp.wasAddColumnHandlerCalled()); - // modify a column family - HColumnDescriptor hcd = new HColumnDescriptor(TEST_FAMILY2); - hcd.setMaxVersions(25); - admin.modifyColumnFamily(tableName, hcd); - assertTrue("Second column family should be modified", + // modify a column family + HColumnDescriptor hcd = new HColumnDescriptor(TEST_FAMILY2); + hcd.setMaxVersions(25); + admin.modifyColumnFamily(tableName, hcd); + assertTrue("Second column family should be modified", cp.wasModifyColumnCalled()); - assertTrue("Modify table handler should be called.", + assertTrue("Modify table handler should be called.", cp.wasModifyColumnHandlerCalled()); - // enable - assertFalse(cp.wasEnableTableCalled()); - assertFalse(cp.wasEnableTableHandlerCalled()); - admin.enableTable(tableName); - assertTrue(admin.isTableEnabled(tableName)); - assertTrue("Coprocessor should have been called on table enable", + // enable + assertFalse(cp.wasEnableTableCalled()); + assertFalse(cp.wasEnableTableHandlerCalled()); + admin.enableTable(tableName); + assertTrue(admin.isTableEnabled(tableName)); + assertTrue("Coprocessor should have been called on table enable", cp.wasEnableTableCalled()); - assertTrue("Enable table handler should be called.", + assertTrue("Enable table handler should be called.", cp.wasEnableTableHandlerCalled()); - // disable again - admin.disableTable(tableName); - assertTrue(admin.isTableDisabled(tableName)); + // disable again + admin.disableTable(tableName); + assertTrue(admin.isTableDisabled(tableName)); - // delete column - assertFalse("No column family deleted yet", cp.wasDeleteColumnCalled()); - assertFalse("Delete table column handler should not be called.", + // delete column + assertFalse("No column family deleted yet", cp.wasDeleteColumnCalled()); + assertFalse("Delete table column handler should not be called.", cp.wasDeleteColumnHandlerCalled()); - admin.deleteColumnFamily(tableName, TEST_FAMILY2); - HTableDescriptor tableDesc = admin.getTableDescriptor(tableName); - assertNull("'"+Bytes.toString(TEST_FAMILY2)+"' should have been removed", + admin.deleteColumnFamily(tableName, TEST_FAMILY2); + HTableDescriptor tableDesc = admin.getTableDescriptor(tableName); + assertNull("'"+Bytes.toString(TEST_FAMILY2)+"' should have been removed", tableDesc.getFamily(TEST_FAMILY2)); - assertTrue("Coprocessor should have been called on column delete", + assertTrue("Coprocessor should have been called on column delete", cp.wasDeleteColumnCalled()); - assertTrue("Delete table column handler should be called.", + assertTrue("Delete table column handler should be called.", cp.wasDeleteColumnHandlerCalled()); - // delete table - assertFalse("No table deleted yet", cp.wasDeleteTableCalled()); - assertFalse("Delete table handler should not be called.", + // delete table + assertFalse("No table deleted yet", cp.wasDeleteTableCalled()); + assertFalse("Delete table handler should not be called.", cp.wasDeleteTableHandlerCalled()); - deleteTable(admin, tableName); - assertFalse("Test table should have been deleted", + deleteTable(admin, tableName); + assertFalse("Test table should have been deleted", admin.tableExists(tableName)); - assertTrue("Coprocessor should have been called on table delete", + assertTrue("Coprocessor should have been called on table delete", cp.wasDeleteTableCalled()); - assertTrue("Delete table handler should be called.", + assertTrue("Delete table handler should be called.", cp.wasDeleteTableHandlerCalled()); + } } @Test (timeout=180000)