hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From apurt...@apache.org
Subject [1/3] hbase git commit: HBASE-12575 Sanity check table coprocessor classes are loadable
Date Tue, 09 Dec 2014 02:31:44 GMT
Repository: hbase
Updated Branches:
  refs/heads/0.98 a213c4a78 -> 8569f9744
  refs/heads/branch-1 1d880e3f6 -> a731ea630
  refs/heads/master a8e646185 -> b4371252f


HBASE-12575 Sanity check table coprocessor classes are loadable


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

Branch: refs/heads/master
Commit: b4371252fe71db938b37b7bc6391d0fb87466b36
Parents: a8e6461
Author: Andrew Purtell <apurtell@apache.org>
Authored: Mon Dec 8 18:18:22 2014 -0800
Committer: Andrew Purtell <apurtell@apache.org>
Committed: Mon Dec 8 18:18:22 2014 -0800

----------------------------------------------------------------------
 .../org/apache/hadoop/hbase/master/HMaster.java |  11 +-
 .../hadoop/hbase/regionserver/HRegion.java      |  10 ++
 .../regionserver/RegionCoprocessorHost.java     | 126 +++++++++++++++----
 hbase-shell/src/test/ruby/hbase/admin_test.rb   |  22 ++--
 4 files changed, 131 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/b4371252/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 3e96c54..af4abb0 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
@@ -106,6 +106,7 @@ import org.apache.hadoop.hbase.protobuf.generated.ZooKeeperProtos.SplitLogTask.R
 import org.apache.hadoop.hbase.quotas.MasterQuotaManager;
 import org.apache.hadoop.hbase.regionserver.HRegionServer;
 import org.apache.hadoop.hbase.regionserver.RSRpcServices;
+import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost;
 import org.apache.hadoop.hbase.regionserver.RegionSplitPolicy;
 import org.apache.hadoop.hbase.replication.regionserver.Replication;
 import org.apache.hadoop.hbase.security.UserProvider;
@@ -1224,9 +1225,9 @@ public class HMaster extends HRegionServer implements MasterServices,
Server {
           + "if you want to bypass sanity checks");
     }
 
-    // check split policy class can be loaded
+    // check that coprocessors and other specified plugin classes can be loaded
     try {
-      RegionSplitPolicy.getSplitPolicyClass(htd, conf);
+      checkClassLoading(conf, htd);
     } catch (Exception ex) {
       throw new DoNotRetryIOException(ex);
     }
@@ -1379,6 +1380,12 @@ public class HMaster extends HRegionServer implements MasterServices,
Server {
     EncryptionTest.testEncryption(conf, hcd.getEncryptionType(), hcd.getEncryptionKey());
   }
 
+  private void checkClassLoading(final Configuration conf, final HTableDescriptor htd)
+  throws IOException {
+    RegionSplitPolicy.getSplitPolicyClass(htd, conf);
+    RegionCoprocessorHost.testTableCoprocessorAttrs(conf, htd);
+  }
+
   private HRegionInfo[] getHRegionInfos(HTableDescriptor hTableDescriptor,
     byte[][] splitKeys) {
     long regionId = System.currentTimeMillis();

http://git-wip-us.apache.org/repos/asf/hbase/blob/b4371252/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index fd5225c..97360d2 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -4853,8 +4853,13 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver
{ //
    */
   protected HRegion openHRegion(final CancelableProgressable reporter)
   throws IOException {
+    // Refuse to open the region if we are missing local compression support
     checkCompressionCodecs();
+    // Refuse to open the region if encryption configuration is incorrect or
+    // codec support is missing
     checkEncryption();
+    // Refuse to open the region if a required class cannot be loaded
+    checkClassLoading();
     this.openSeqNum = initialize(reporter);
     this.setSequenceId(openSeqNum);
     if (wal != null && getRegionServerServices() != null) {
@@ -4876,6 +4881,11 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver
{ //
     }
   }
 
+  private void checkClassLoading() throws IOException {
+    RegionSplitPolicy.getSplitPolicyClass(this.htableDescriptor, conf);
+    RegionCoprocessorHost.testTableCoprocessorAttrs(conf, this.htableDescriptor);
+  }
+
   /**
    * Create a daughter region from given a temp directory with the region data.
    * @param hri Spec. for daughter region to open.

http://git-wip-us.apache.org/repos/asf/hbase/blob/b4371252/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
index e671e50..87c8b9e 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
@@ -26,6 +26,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.NavigableSet;
+import java.util.UUID;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ConcurrentHashMap;
@@ -36,6 +37,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.protobuf.Message;
 import com.google.protobuf.Service;
+
 import org.apache.commons.collections.map.AbstractReferenceMap;
 import org.apache.commons.collections.map.ReferenceMap;
 import org.apache.commons.logging.Log;
@@ -53,6 +55,7 @@ import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.client.Append;
 import org.apache.hadoop.hbase.client.Delete;
 import org.apache.hadoop.hbase.client.Durability;
@@ -80,6 +83,7 @@ import org.apache.hadoop.hbase.regionserver.wal.HLogKey;
 import org.apache.hadoop.hbase.wal.WALKey;
 import org.apache.hadoop.hbase.regionserver.wal.WALEdit;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.CoprocessorClassLoader;
 import org.apache.hadoop.hbase.util.Pair;
 
 /**
@@ -166,6 +170,37 @@ public class RegionCoprocessorHost
 
   }
 
+  static class TableCoprocessorAttribute {
+    private Path path;
+    private String className;
+    private int priority;
+    private Configuration conf;
+
+    public TableCoprocessorAttribute(Path path, String className, int priority,
+        Configuration conf) {
+      this.path = path;
+      this.className = className;
+      this.priority = priority;
+      this.conf = conf;
+    }
+
+    public Path getPath() {
+      return path;
+    }
+
+    public String getClassName() {
+      return className;
+    }
+
+    public int getPriority() {
+      return priority;
+    }
+
+    public Configuration getConf() {
+      return conf;
+    }
+  }
+
   /** The region server services */
   RegionServerServices rsServices;
   /** The region */
@@ -197,15 +232,13 @@ public class RegionCoprocessorHost
     loadTableCoprocessors(conf);
   }
 
-  void loadTableCoprocessors(final Configuration conf) {
-    // scan the table attributes for coprocessor load specifications
-    // initialize the coprocessors
-    List<RegionEnvironment> configured = new ArrayList<RegionEnvironment>();
-    for (Map.Entry<Bytes, Bytes> e :
-        region.getTableDesc().getValues().entrySet()) {
+  static List<TableCoprocessorAttribute> getTableCoprocessorAttrsFromSchema(Configuration
conf,
+      HTableDescriptor htd) {
+    List<TableCoprocessorAttribute> result = Lists.newArrayList();
+    for (Map.Entry<Bytes, Bytes> e: htd.getValues().entrySet()) {
       String key = Bytes.toString(e.getKey().get()).trim();
-      String spec = Bytes.toString(e.getValue().get()).trim();
       if (HConstants.CP_HTD_ATTR_KEY_PATTERN.matcher(key).matches()) {
+        String spec = Bytes.toString(e.getValue().get()).trim();
         // found one
         try {
           Matcher matcher = HConstants.CP_HTD_ATTR_VALUE_PATTERN.matcher(spec);
@@ -215,6 +248,11 @@ public class RegionCoprocessorHost
             Path path = matcher.group(1).trim().isEmpty() ?
                 null : new Path(matcher.group(1).trim());
             String className = matcher.group(2).trim();
+            if (className.isEmpty()) {
+              LOG.error("Malformed table coprocessor specification: key=" +
+                key + ", spec: " + spec);
+              continue;
+            }
             int priority = matcher.group(3).trim().isEmpty() ?
                 Coprocessor.PRIORITY_USER : Integer.valueOf(matcher.group(3));
             String cfgSpec = null;
@@ -236,20 +274,7 @@ public class RegionCoprocessorHost
             } else {
               ourConf = conf;
             }
-            // Load encompasses classloading and coprocessor initialization
-            try {
-              RegionEnvironment env = load(path, className, priority, ourConf);
-              configured.add(env);
-              LOG.info("Loaded coprocessor " + className + " from HTD of " +
-                region.getTableDesc().getTableName().getNameAsString() + " successfully.");
-            } catch (Throwable t) {
-              // Coprocessor failed to load, do we abort on error?
-              if (conf.getBoolean(ABORT_ON_ERROR_KEY, DEFAULT_ABORT_ON_ERROR)) {
-                abortServer(className, t);
-              } else {
-                LOG.error("Failed to load coprocessor " + className, t);
-              }
-            }
+            result.add(new TableCoprocessorAttribute(path, className, priority, ourConf));
           } else {
             LOG.error("Malformed table coprocessor specification: key=" + key +
               ", spec: " + spec);
@@ -260,6 +285,65 @@ public class RegionCoprocessorHost
         }
       }
     }
+    return result;
+  }
+
+  /**
+   * Sanity check the table coprocessor attributes of the supplied schema. Will
+   * throw an exception if there is a problem.
+   * @param conf
+   * @param htd
+   * @throws IOException
+   */
+  public static void testTableCoprocessorAttrs(final Configuration conf,
+      final HTableDescriptor htd) throws IOException {
+    String pathPrefix = UUID.randomUUID().toString();
+    for (TableCoprocessorAttribute attr: getTableCoprocessorAttrsFromSchema(conf, htd)) {
+      if (attr.getPriority() < 0) {
+        throw new IOException("Priority for coprocessor " + attr.getClassName() +
+          " cannot be less than 0");
+      }
+      ClassLoader old = Thread.currentThread().getContextClassLoader();
+      try {
+        ClassLoader cl;
+        if (attr.getPath() != null) {
+          cl = CoprocessorClassLoader.getClassLoader(attr.getPath(),
+            CoprocessorHost.class.getClassLoader(), pathPrefix, conf);
+        } else {
+          cl = CoprocessorHost.class.getClassLoader();
+        }
+        Thread.currentThread().setContextClassLoader(cl);
+        cl.loadClass(attr.getClassName());
+      } catch (ClassNotFoundException e) {
+        throw new IOException("Class " + attr.getClassName() + " cannot be loaded", e);
+      } finally {
+        Thread.currentThread().setContextClassLoader(old);
+      }
+    }
+  }
+
+  void loadTableCoprocessors(final Configuration conf) {
+    // scan the table attributes for coprocessor load specifications
+    // initialize the coprocessors
+    List<RegionEnvironment> configured = new ArrayList<RegionEnvironment>();
+    for (TableCoprocessorAttribute attr: getTableCoprocessorAttrsFromSchema(conf, 
+        region.getTableDesc())) {
+      // Load encompasses classloading and coprocessor initialization
+      try {
+        RegionEnvironment env = load(attr.getPath(), attr.getClassName(), attr.getPriority(),
+          attr.getConf());
+        configured.add(env);
+        LOG.info("Loaded coprocessor " + attr.getClassName() + " from HTD of " +
+            region.getTableDesc().getTableName().getNameAsString() + " successfully.");
+      } catch (Throwable t) {
+        // Coprocessor failed to load, do we abort on error?
+        if (conf.getBoolean(ABORT_ON_ERROR_KEY, DEFAULT_ABORT_ON_ERROR)) {
+          abortServer(attr.getClassName(), t);
+        } else {
+          LOG.error("Failed to load coprocessor " + attr.getClassName(), t);
+        }
+      }
+    }
     // add together to coprocessor set for COW efficiency
     coprocessors.addAll(configured);
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/b4371252/hbase-shell/src/test/ruby/hbase/admin_test.rb
----------------------------------------------------------------------
diff --git a/hbase-shell/src/test/ruby/hbase/admin_test.rb b/hbase-shell/src/test/ruby/hbase/admin_test.rb
index ff2f422..0b12df9 100644
--- a/hbase-shell/src/test/ruby/hbase/admin_test.rb
+++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb
@@ -310,9 +310,9 @@ module Hbase
       create_test_table(@test_name)
 
       cp_key = "coprocessor"
-      class_name = "SimpleRegionObserver"
+      class_name = "org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver"
 
-      cp_value = "hdfs:///foo.jar|" + class_name + "|12|arg1=1,arg2=2"
+      cp_value = "|" + class_name + "|12|arg1=1,arg2=2"
 
       # eval() is used to convert a string to regex
       assert_no_match(eval("/" + class_name + "/"), admin.describe(@test_name))
@@ -326,22 +326,14 @@ module Hbase
       drop_test_table(@test_name)
       create_test_table(@test_name)
 
-      key1 = "coprocessor"
-      key2 = "MAX_FILESIZE"
-      admin.alter(@test_name, true, 'METHOD' => 'table_att', key1 => "|TestCP||")
-      admin.alter(@test_name, true, 'METHOD' => 'table_att', key2 => 12345678)
+      key = "MAX_FILESIZE"
+      admin.alter(@test_name, true, 'METHOD' => 'table_att', key => 12345678)
 
       # eval() is used to convert a string to regex
-      assert_match(eval("/" + key1 + "\\$(\\d+)/"), admin.describe(@test_name))
-      assert_match(eval("/" + key2 + "/"), admin.describe(@test_name))
+      assert_match(eval("/" + key + "/"), admin.describe(@test_name))
 
-      # get the cp key
-      cp_keys = admin.describe(@test_name).scan(/(coprocessor\$\d+)/i)
-
-      admin.alter(@test_name, true, 'METHOD' => 'table_att_unset', 'NAME' => cp_keys[0][0])
-      admin.alter(@test_name, true, 'METHOD' => 'table_att_unset', 'NAME' => key2)
-      assert_no_match(eval("/" + key1 + "\\$(\\d+)/"), admin.describe(@test_name))
-      assert_no_match(eval("/" + key2 + "/"), admin.describe(@test_name))
+      admin.alter(@test_name, true, 'METHOD' => 'table_att_unset', 'NAME' => key)
+      assert_no_match(eval("/" + key + "/"), admin.describe(@test_name))
     end
 
     define_test "get_table should get a real table" do


Mime
View raw message