You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zg...@apache.org on 2018/12/08 01:28:45 UTC

hbase git commit: HBASE-21560 Return a new TableDescriptor for MasterObserver#preModifyTable to allow coprocessor modify the TableDescriptor

Repository: hbase
Updated Branches:
  refs/heads/master 8d7061a48 -> 79d90c87b


HBASE-21560 Return a new TableDescriptor for MasterObserver#preModifyTable to allow coprocessor modify the TableDescriptor


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

Branch: refs/heads/master
Commit: 79d90c87b5bc6d4aa50e6edc52a3f20da708ee29
Parents: 8d7061a
Author: Guanghao Zhang <zg...@apache.org>
Authored: Fri Dec 7 16:51:19 2018 +0800
Committer: Guanghao Zhang <zg...@apache.org>
Committed: Sat Dec 8 09:28:14 2018 +0800

----------------------------------------------------------------------
 .../hbase/coprocessor/MasterObserver.java       |   6 +-
 .../org/apache/hadoop/hbase/master/HMaster.java |  11 +-
 .../hbase/master/MasterCoprocessorHost.java     |  22 ++--
 .../hbase/security/access/AccessController.java |   9 +-
 .../CoprocessorWhitelistMasterObserver.java     |   5 +-
 .../visibility/VisibilityController.java        |  15 ++-
 .../hbase/coprocessor/TestMasterObserver.java   |   3 +-
 .../TestMasterObserverToModifyTableSchema.java  | 128 +++++++++++++++++++
 8 files changed, 169 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
----------------------------------------------------------------------
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 a0863e4..1a8db79 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
@@ -240,11 +240,13 @@ public interface MasterObserver {
    * @param currentDescriptor current TableDescriptor of the table
    * @param newDescriptor after modify operation, table will have this descriptor
    */
-  default void preModifyTable(final ObserverContext<MasterCoprocessorEnvironment> ctx,
+  default TableDescriptor preModifyTable(final ObserverContext<MasterCoprocessorEnvironment> ctx,
       final TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor)
-    throws IOException {
+      throws IOException {
     preModifyTable(ctx, tableName, newDescriptor);
+    return newDescriptor;
   }
+
   /**
    * Called after the modifyTable operation has been requested.  Called as part
    * of modify table RPC call.

http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
----------------------------------------------------------------------
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 e96dc36..a16e09d 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
@@ -2631,13 +2631,12 @@ public class HMaster extends HRegionServer implements MasterServices {
         .submitProcedure(new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) {
           @Override
           protected void run() throws IOException {
-            TableDescriptor newDescriptor = newDescriptorGetter.get();
-            sanityCheckTableDescriptor(newDescriptor);
             TableDescriptor oldDescriptor = getMaster().getTableDescriptors().get(tableName);
-            getMaster().getMasterCoprocessorHost().preModifyTable(tableName, oldDescriptor,
-              newDescriptor);
-
-            LOG.info(getClientIdAuditPrefix() + " modify " + tableName);
+            TableDescriptor newDescriptor = getMaster().getMasterCoprocessorHost()
+                .preModifyTable(tableName, oldDescriptor, newDescriptorGetter.get());
+            sanityCheckTableDescriptor(newDescriptor);
+            LOG.info("{} modify table {} from {} to {}", getClientIdAuditPrefix(), tableName,
+                oldDescriptor, newDescriptor);
 
             // Execute the operation synchronously - wait for the operation completes before
             // continuing.

http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
----------------------------------------------------------------------
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 51e30c4..e7b166c 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
@@ -446,14 +446,20 @@ public class MasterCoprocessorHost
     });
   }
 
-  public void preModifyTable(final TableName tableName, final TableDescriptor currentDescriptor,
-    final TableDescriptor newDescriptor) throws IOException {
-    execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() {
-      @Override
-      public void call(MasterObserver observer) throws IOException {
-        observer.preModifyTable(this, tableName, currentDescriptor, newDescriptor);
-      }
-    });
+  public TableDescriptor preModifyTable(final TableName tableName,
+      final TableDescriptor currentDescriptor, final TableDescriptor newDescriptor)
+      throws IOException {
+    if (coprocEnvironments.isEmpty()) {
+      return newDescriptor;
+    }
+    return execOperationWithResult(
+        new ObserverOperationWithResult<MasterObserver, TableDescriptor>(masterObserverGetter,
+            newDescriptor) {
+          @Override
+          protected TableDescriptor call(MasterObserver observer) throws IOException {
+            return observer.preModifyTable(this, tableName, currentDescriptor, getResult());
+          }
+        });
   }
 
   public void postModifyTable(final TableName tableName, final TableDescriptor oldDescriptor,

http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
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 835fc0d..82ec12d 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
@@ -970,11 +970,12 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
   }
 
   @Override
-  public void preModifyTable(ObserverContext<MasterCoprocessorEnvironment> c, TableName tableName,
-      TableDescriptor currentDesc, TableDescriptor newDesc) throws IOException {
+  public TableDescriptor preModifyTable(ObserverContext<MasterCoprocessorEnvironment> c,
+      TableName tableName, TableDescriptor currentDesc, TableDescriptor newDesc)
+      throws IOException {
     // TODO: potentially check if this is a add/modify/delete column operation
-    requirePermission(c, "modifyTable",
-        tableName, null, null, Action.ADMIN, Action.CREATE);
+    requirePermission(c, "modifyTable", tableName, null, null, Action.ADMIN, Action.CREATE);
+    return newDesc;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java
index 719fe33..1e83e96 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java
@@ -54,10 +54,11 @@ public class CoprocessorWhitelistMasterObserver implements MasterCoprocessor, Ma
   }
 
   @Override
-  public void preModifyTable(ObserverContext<MasterCoprocessorEnvironment> ctx,
+  public TableDescriptor preModifyTable(ObserverContext<MasterCoprocessorEnvironment> ctx,
       TableName tableName, TableDescriptor currentDesc, TableDescriptor newDesc)
-    throws IOException {
+      throws IOException {
     verifyCoprocessors(ctx, newDesc);
+    return newDesc;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
index 1f54afc..c4f3b95 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
@@ -226,14 +226,15 @@ public class VisibilityController implements MasterCoprocessor, RegionCoprocesso
   }
 
   @Override
-  public void preModifyTable(ObserverContext<MasterCoprocessorEnvironment> ctx, TableName tableName,
-    TableDescriptor currentDescriptor, TableDescriptor newDescriptor) throws IOException {
-    if (!authorizationEnabled) {
-      return;
-    }
-    if (LABELS_TABLE_NAME.equals(tableName)) {
-      throw new ConstraintException("Cannot alter " + LABELS_TABLE_NAME);
+  public TableDescriptor preModifyTable(ObserverContext<MasterCoprocessorEnvironment> ctx,
+      TableName tableName, TableDescriptor currentDescriptor, TableDescriptor newDescriptor)
+      throws IOException {
+    if (authorizationEnabled) {
+      if (LABELS_TABLE_NAME.equals(tableName)) {
+        throw new ConstraintException("Cannot alter " + LABELS_TABLE_NAME);
+      }
     }
+    return newDescriptor;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java
----------------------------------------------------------------------
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 d8a5b4c..58f4b9b 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
@@ -373,10 +373,11 @@ public class TestMasterObserver {
     }
 
     @Override
-    public void preModifyTable(ObserverContext<MasterCoprocessorEnvironment> env,
+    public TableDescriptor preModifyTable(ObserverContext<MasterCoprocessorEnvironment> env,
         TableName tableName, final TableDescriptor currentDescriptor,
       final TableDescriptor newDescriptor) throws IOException {
       preModifyTableCalled = true;
+      return newDescriptor;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/79d90c87/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserverToModifyTableSchema.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserverToModifyTableSchema.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserverToModifyTableSchema.java
new file mode 100644
index 0000000..d23a4a8
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserverToModifyTableSchema.java
@@ -0,0 +1,128 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.coprocessor;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.testclassification.CoprocessorTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+@Category({ CoprocessorTests.class, MediumTests.class })
+public class TestMasterObserverToModifyTableSchema {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+      HBaseClassTestRule.forClass(TestMasterObserverToModifyTableSchema.class);
+
+  private static HBaseTestingUtility UTIL = new HBaseTestingUtility();
+  private static TableName TABLENAME = TableName.valueOf("TestTable");
+
+  @Rule
+  public TestName name = new TestName();
+
+  @BeforeClass
+  public static void setupBeforeClass() throws Exception {
+    Configuration conf = UTIL.getConfiguration();
+    conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY,
+        OnlyOneVersionAllowedMasterObserver.class.getName());
+    UTIL.startMiniCluster(1);
+  }
+
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+    UTIL.shutdownMiniCluster();
+  }
+
+  @Test
+  public void testMasterObserverToModifyTableSchema() throws IOException {
+    TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(TABLENAME);
+    for (int i = 1; i <= 3; i++) {
+      builder.setColumnFamily(
+          ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("cf" + i)).setMaxVersions(i)
+              .build());
+    }
+    try (Admin admin = UTIL.getAdmin()) {
+      admin.createTable(builder.build());
+      assertOneVersion(admin.getDescriptor(TABLENAME));
+
+      builder.modifyColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(Bytes.toBytes("cf1"))
+          .setMaxVersions(Integer.MAX_VALUE).build());
+      admin.modifyTable(builder.build());
+      assertOneVersion(admin.getDescriptor(TABLENAME));
+    }
+  }
+
+  private void assertOneVersion(TableDescriptor td) {
+    for (ColumnFamilyDescriptor cfd : td.getColumnFamilies()) {
+      assertEquals(1, cfd.getMaxVersions());
+    }
+  }
+
+  public static class OnlyOneVersionAllowedMasterObserver
+      implements MasterCoprocessor, MasterObserver {
+
+    @Override
+    public Optional<MasterObserver> getMasterObserver() {
+      return Optional.of(this);
+    }
+
+    @Override
+    public TableDescriptor preCreateTableRegionsInfos(
+        ObserverContext<MasterCoprocessorEnvironment> ctx, TableDescriptor desc)
+        throws IOException {
+      TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(desc);
+      for (ColumnFamilyDescriptor cfd : desc.getColumnFamilies()) {
+        builder.modifyColumnFamily(
+            ColumnFamilyDescriptorBuilder.newBuilder(cfd).setMaxVersions(1).build());
+      }
+      return builder.build();
+    }
+
+    @Override
+    public TableDescriptor preModifyTable(ObserverContext<MasterCoprocessorEnvironment> env,
+        TableName tableName, final TableDescriptor currentDescriptor,
+        final TableDescriptor newDescriptor) throws IOException {
+      TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(newDescriptor);
+      for (ColumnFamilyDescriptor cfd : newDescriptor.getColumnFamilies()) {
+        builder.modifyColumnFamily(
+            ColumnFamilyDescriptorBuilder.newBuilder(cfd).setMaxVersions(1).build());
+      }
+      return builder.build();
+    }
+  }
+}