From b85aa185a6d3c560bb668a0a99264d98bd986d47 Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Wed, 26 Jan 2022 14:04:26 -0800 Subject: [PATCH] HBASE-26714 Introduce path configuration for system coprocessors --- .../hbase/util/ClassLoaderTestHelper.java | 2 +- .../hbase/coprocessor/CoprocessorHost.java | 24 +++- .../coprocessor/TestCoprocessorHost.java | 116 +++++++++++++++++- 3 files changed, 135 insertions(+), 7 deletions(-) diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/ClassLoaderTestHelper.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/ClassLoaderTestHelper.java index fd771c722b88..decc0da2e9b4 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/ClassLoaderTestHelper.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/ClassLoaderTestHelper.java @@ -213,7 +213,7 @@ public static void addJarFilesToJar(File targetJar, LOG.info("Adding jar file to outer jar file completed"); } - static String localDirPath(Configuration conf) { + public static String localDirPath(Configuration conf) { return conf.get(ClassLoaderBase.LOCAL_DIR_KEY) + File.separator + "jars" + File.separator; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java index 442507630056..319936d9ebfe 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java @@ -46,6 +46,7 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.CoprocessorClassLoader; import org.apache.hadoop.hbase.util.SortedList; +import org.apache.hbase.thirdparty.com.google.common.base.Strings; /** * Provides the common setup framework and runtime services for coprocessor @@ -144,14 +145,22 @@ protected void loadSystemCoprocessors(Configuration conf, String confKey) { int currentSystemPriority = Coprocessor.PRIORITY_SYSTEM; for (String className : defaultCPClasses) { - String[] classNameAndPriority = className.split("\\|"); + // After HBASE-23710 and HBASE-26714 when configuring for system coprocessor, we accept + // an optional format of className|priority|path + String[] classNameToken = className.split("\\|"); boolean hasPriorityOverride = false; - className = classNameAndPriority[0]; + boolean hasPath = false; + className = classNameToken[0]; int overridePriority = Coprocessor.PRIORITY_SYSTEM; - if (classNameAndPriority.length > 1){ - overridePriority = Integer.parseInt(classNameAndPriority[1]); + Path path = null; + if (classNameToken.length > 1 && !Strings.isNullOrEmpty(classNameToken[1])) { + overridePriority = Integer.parseInt(classNameToken[1]); hasPriorityOverride = true; } + if (classNameToken.length > 2 && !Strings.isNullOrEmpty(classNameToken[2])) { + path = new Path(classNameToken[2].trim()); + hasPath = true; + } className = className.trim(); if (findCoprocessor(className) != null) { // If already loaded will just continue @@ -159,8 +168,13 @@ protected void loadSystemCoprocessors(Configuration conf, String confKey) { continue; } ClassLoader cl = this.getClass().getClassLoader(); - Thread.currentThread().setContextClassLoader(cl); try { + // override the class loader if a path for the system coprocessor is provided. + if (hasPath) { + cl = CoprocessorClassLoader.getClassLoader(path, this.getClass().getClassLoader(), + pathPrefix, conf); + } + Thread.currentThread().setContextClassLoader(cl); implClass = cl.loadClass(className); int coprocPriority = hasPriorityOverride ? overridePriority : currentSystemPriority; // Add coprocessors as we go to guard against case where a coprocessor is specified twice diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java index a25c2a6980a2..9531a7d045bc 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import java.io.File; import java.lang.reflect.InvocationTargetException; import org.apache.hadoop.conf.Configuration; @@ -27,8 +28,10 @@ import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseCommonTestingUtil; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.ClassLoaderTestHelper; import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; @@ -41,6 +44,8 @@ public class TestCoprocessorHost { public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestCoprocessorHost.class); + private static final HBaseCommonTestingUtil TEST_UTIL = new HBaseCommonTestingUtil(); + /** * An {@link Abortable} implementation for tests. */ @@ -50,7 +55,7 @@ private static class TestAbortable implements Abortable { @Override public void abort(String why, Throwable e) { this.aborted = true; - Assert.fail(); + Assert.fail(e.getMessage()); } @Override @@ -97,6 +102,108 @@ public void testDoubleLoadingAndPriorityValue() { assertEquals(overridePriority, simpleEnv_v3.getPriority()); } + @Test + public void testLoadSystemCoprocessorWithPath() throws Exception { + Configuration conf = TEST_UTIL.getConfiguration(); + final String key = "KEY"; + final String testClassName = "TestSystemCoprocessor"; + final String testClassNameWithPriorityAndPath = testClassName + "PriorityAndPath"; + + File jarFile = buildCoprocessorJar(testClassName); + File jarFileWithPriorityAndPath = buildCoprocessorJar(testClassNameWithPriorityAndPath); + + try { + CoprocessorHost> host; + host = new CoprocessorHostForTest<>(conf); + + // make a string of coprocessor with only priority + int overridePriority = Integer.MAX_VALUE - 1; + final String coprocessorWithPriority = + SimpleRegionObserverV3.class.getName() + "|" + overridePriority; + // make a string of coprocessor with path but no priority + final String coprocessorWithPath = + String.format("%s|%s|%s", testClassName, "", jarFile.getAbsolutePath()); + // make a string of coprocessor with priority and path + final String coprocessorWithPriorityAndPath = String + .format("%s|%s|%s", testClassNameWithPriorityAndPath, (overridePriority - 1), + jarFileWithPriorityAndPath.getAbsolutePath()); + + // Try and load a system coprocessors + conf.setStrings(key, SimpleRegionObserverV2.class.getName(), coprocessorWithPriority, + coprocessorWithPath, coprocessorWithPriorityAndPath); + host.loadSystemCoprocessors(conf, key); + + // first loaded system coprocessor with default priority + CoprocessorEnvironment simpleEnv = + host.findCoprocessorEnvironment(SimpleRegionObserverV2.class.getName()); + assertNotNull(simpleEnv); + assertEquals(Coprocessor.PRIORITY_SYSTEM, simpleEnv.getPriority()); + + // external system coprocessor with default priority + CoprocessorEnvironment coprocessorEnvironmentWithPath = + host.findCoprocessorEnvironment(testClassName); + assertNotNull(coprocessorEnvironmentWithPath); + assertEquals(Coprocessor.PRIORITY_SYSTEM + 1, coprocessorEnvironmentWithPath.getPriority()); + + // system coprocessor with configured priority + CoprocessorEnvironment coprocessorEnvironmentWithPriority = + host.findCoprocessorEnvironment(SimpleRegionObserverV3.class.getName()); + assertNotNull(coprocessorEnvironmentWithPriority); + assertEquals(overridePriority, coprocessorEnvironmentWithPriority.getPriority()); + + // external system coprocessor with override priority + CoprocessorEnvironment coprocessorEnvironmentWithPriorityAndPath = + host.findCoprocessorEnvironment(testClassNameWithPriorityAndPath); + assertNotNull(coprocessorEnvironmentWithPriorityAndPath); + assertEquals(overridePriority - 1, coprocessorEnvironmentWithPriorityAndPath.getPriority()); + } finally { + if (jarFile.exists()) { + jarFile.delete(); + } + if (jarFileWithPriorityAndPath.exists()) { + jarFileWithPriorityAndPath.delete(); + } + } + } + + @Test(expected = AssertionError.class) + public void testLoadSystemCoprocessorWithPathDoesNotExist() throws Exception { + Configuration conf = TEST_UTIL.getConfiguration(); + final String key = "KEY"; + final String testClassName = "TestSystemCoprocessor"; + + CoprocessorHost> host; + host = new CoprocessorHostForTest<>(conf); + + // make a string of coprocessor with path but no priority + final String coprocessorWithPath = testClassName + "||" + testClassName + ".jar"; + + // Try and load a system coprocessors + conf.setStrings(key, coprocessorWithPath); + // when loading non-exist with CoprocessorHostForTest host, it aborts with AssertionError + host.loadSystemCoprocessors(conf, key); + } + + @Test(expected = AssertionError.class) + public void testLoadSystemCoprocessorWithPathDoesNotExistAndPriority() throws Exception { + Configuration conf = TEST_UTIL.getConfiguration(); + final String key = "KEY"; + final String testClassName = "TestSystemCoprocessor"; + + CoprocessorHost> host; + host = new CoprocessorHostForTest<>(conf); + + int overridePriority = Integer.MAX_VALUE - 1; + // make a string of coprocessor with path and priority + final String coprocessor = + testClassName + "|" + overridePriority + "|" + testClassName + ".jar"; + + // Try and load a system coprocessors + conf.setStrings(key, coprocessor); + // when loading non-exist coprocessor, it aborts with AssertionError + host.loadSystemCoprocessors(conf, key); + } + public static class SimpleRegionObserverV2 extends SimpleRegionObserver { } public static class SimpleRegionObserverV3 extends SimpleRegionObserver { @@ -128,4 +235,11 @@ public CoprocessorEnvironment createEnvironment(final E instance, final int p return new BaseEnvironment<>(instance, priority, 0, cpHostConf); } } + + private File buildCoprocessorJar(String className) throws Exception { + String dataTestDir = TEST_UTIL.getDataTestDir().toString(); + String code = String.format("import org.apache.hadoop.hbase.coprocessor.*; public class %s" + + " implements RegionCoprocessor {}", className); + return ClassLoaderTestHelper.buildJar(dataTestDir, className, code); + } }