diff --git a/common/src/main/java/org/apache/drill/common/util/GuavaPatcher.java b/common/src/main/java/org/apache/drill/common/util/GuavaPatcher.java deleted file mode 100644 index ec6c1cabfc8..00000000000 --- a/common/src/main/java/org/apache/drill/common/util/GuavaPatcher.java +++ /dev/null @@ -1,146 +0,0 @@ -/* - * 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.drill.common.util; - -import java.lang.reflect.Modifier; - -import com.google.common.annotations.VisibleForTesting; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javassist.ClassPool; -import javassist.CtClass; -import javassist.CtConstructor; -import javassist.CtMethod; -import javassist.CtNewMethod; -import javassist.scopedpool.ScopedClassPoolRepository; -import javassist.scopedpool.ScopedClassPoolRepositoryImpl; - -public class GuavaPatcher { - private static final Logger logger = LoggerFactory.getLogger(GuavaPatcher.class); - - private static boolean patchingAttempted; - - public static synchronized void patch() { - if (!patchingAttempted) { - patchingAttempted = true; - patchStopwatch(); - patchCloseables(); - patchFuturesCallback(); - } - } - - private static boolean patchingSuccessful = true; - - @VisibleForTesting - static boolean isPatchingSuccessful() { - return patchingAttempted && patchingSuccessful; - } - - /** - * Makes Guava stopwatch look like the old version for compatibility with hbase-server (for test purposes). - */ - private static void patchStopwatch() { - try { - ClassPool cp = getClassPool(); - CtClass cc = cp.get("com.google.common.base.Stopwatch"); - - // Expose the constructor for Stopwatch for old libraries who use the pattern new Stopwatch().start(). - for (CtConstructor c : cc.getConstructors()) { - if (!Modifier.isStatic(c.getModifiers())) { - c.setModifiers(Modifier.PUBLIC); - } - } - - // Add back the Stopwatch.elapsedMillis() method for old consumers. - CtMethod newMethod = CtNewMethod.make( - "public long elapsedMillis() { return elapsed(java.util.concurrent.TimeUnit.MILLISECONDS); }", cc); - cc.addMethod(newMethod); - - // Load the modified class instead of the original. - cc.toClass(); - - logger.info("Google's Stopwatch patched for old HBase Guava version."); - } catch (Exception e) { - logUnableToPatchException(e); - } - } - - private static void logUnableToPatchException(Exception e) { - patchingSuccessful = false; - logger.warn("Unable to patch Guava classes: {}", e.getMessage()); - logger.debug("Exception:", e); - } - - private static void patchCloseables() { - try { - ClassPool cp = getClassPool(); - CtClass cc = cp.get("com.google.common.io.Closeables"); - - // Add back the Closeables.closeQuietly() method for old consumers. - CtMethod newMethod = CtNewMethod.make( - "public static void closeQuietly(java.io.Closeable closeable) { try{closeable.close();}catch(Exception e){} }", - cc); - cc.addMethod(newMethod); - - // Load the modified class instead of the original. - cc.toClass(); - - logger.info("Google's Closeables patched for old HBase Guava version."); - } catch (Exception e) { - logUnableToPatchException(e); - } - } - - /** - * Returns {@link javassist.scopedpool.ScopedClassPool} instance which uses the same class loader - * which was used for loading current class. - * - * @return {@link javassist.scopedpool.ScopedClassPool} instance - */ - private static ClassPool getClassPool() { - ScopedClassPoolRepository classPoolRepository = ScopedClassPoolRepositoryImpl.getInstance(); - // sets prune flag to false to avoid freezing and pruning classes right after obtaining CtClass instance - classPoolRepository.setPrune(false); - return classPoolRepository.createScopedClassPool(GuavaPatcher.class.getClassLoader(), null); - } - - /** - * Makes Guava Futures#addCallback look like the old version for compatibility with hadoop-hdfs (for test purposes). - */ - private static void patchFuturesCallback() { - try { - ClassPool cp = getClassPool(); - CtClass cc = cp.get("com.google.common.util.concurrent.Futures"); - - // Add back the Futures.addCallback(ListenableFuture future, FutureCallback callback) method for old consumers. - CtMethod newMethod = CtNewMethod.make( - "public static void addCallback(" + - "com.google.common.util.concurrent.ListenableFuture future, " + - "com.google.common.util.concurrent.FutureCallback callback" + - ") { addCallback(future, callback, com.google.common.util.concurrent.MoreExecutors.directExecutor()); }", cc); - cc.addMethod(newMethod); - - // Load the modified class instead of the original. - cc.toClass(); - logger.info("Google's Futures#addCallback patched for old Hadoop Guava version."); - } catch (Exception e) { - logUnableToPatchException(e); - } - } -} diff --git a/common/src/test/java/org/apache/drill/common/util/TestGuavaPatcher.java b/common/src/test/java/org/apache/drill/common/util/TestGuavaPatcher.java deleted file mode 100644 index 5dbdf537e7a..00000000000 --- a/common/src/test/java/org/apache/drill/common/util/TestGuavaPatcher.java +++ /dev/null @@ -1,243 +0,0 @@ -/* - * 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.drill.common.util; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -import java.io.Closeable; -import java.io.IOException; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import org.apache.drill.test.BaseTest; -import org.junit.Test; - -// Easier to test Guava patching if we can reference Guava classes directly for the tests -// CHECKSTYLE:OFF -import com.google.common.base.Preconditions; -import com.google.common.base.Stopwatch; -import com.google.common.base.Ticker; -import com.google.common.io.Closeables; -//CHECKSTYLE:ON - -/** - * Test class for {@code GuavaPatcher} - * - */ -public class TestGuavaPatcher extends BaseTest { - - @Test - public void checkSuccessfulPatching() { - // Catch-all test to see if Guava patching was successful - assertTrue("Guava Patcher ran with errors, check error log messages", - GuavaPatcher.isPatchingSuccessful()); - } - - @Test - public void checkStopwatchEllapsedMillis() throws Exception { - long[] currentTimeMillis = new long[] { 0L }; - final Ticker ticker = new Ticker() { - @Override - public long read() { - return TimeUnit.MILLISECONDS.toNanos(currentTimeMillis[0]); - } - }; - final Stopwatch stopwatch = Stopwatch.createStarted(ticker); - currentTimeMillis[0] = 12345L; - stopwatch.stop(); - - assertEquals(currentTimeMillis[0], - stopwatch.elapsed(TimeUnit.MILLISECONDS)); - assertEquals(currentTimeMillis[0], (long) invokeMethod(Stopwatch.class, - "elapsedMillis", new Class[] {}, stopwatch)); - } - - @Test - public void checkCloseablesCloseQuietly() throws Exception { - final Closeable alwaysThrows = () -> { - throw new IOException("Always fail"); - }; - - invokeMethod(Closeables.class, "closeQuietly", - new Class[] { Closeable.class }, null, alwaysThrows); - } - - // All the preconditions checks are method which were previously added for - // compatibility with Apache Iceberg but are not necessary anymore because - // Guava's version has been updated since. - - @Test - public void checkPreconditionsCheckArgumentIntParam() throws Exception { - invokeMethod(Preconditions.class, "checkArgument", - new Class[] { boolean.class, String.class, int.class }, null, true, - "Error Message %s", 1); - try { - invokeMethod(Preconditions.class, "checkArgument", - new Class[] { boolean.class, String.class, int.class }, null, false, - "Error Message %s", 1); - fail(); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - assertEquals(IllegalArgumentException.class, cause.getClass()); - assertEquals("Error Message 1", cause.getMessage()); - } - } - - @Test - public void checkPreconditionsCheckArgumentLongParam() throws Exception { - invokeMethod(Preconditions.class, "checkArgument", - new Class[] { boolean.class, String.class, long.class }, null, true, - "Error Message %s", 2L); - try { - invokeMethod(Preconditions.class, "checkArgument", - new Class[] { boolean.class, String.class, long.class }, null, false, - "Error Message %s", 2L); - fail(); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - assertEquals(IllegalArgumentException.class, cause.getClass()); - assertEquals("Error Message 2", cause.getMessage()); - } - } - - @Test - public void checkPreconditionsCheckArgumentLongLongParam() throws Exception { - invokeMethod(Preconditions.class, "checkArgument", - new Class[] { boolean.class, String.class, long.class, long.class }, - null, true, "Error Message %s %s", 3L, 4L); - try { - invokeMethod(Preconditions.class, "checkArgument", - new Class[] { boolean.class, String.class, long.class, long.class }, - null, false, "Error Message %s %s", 3L, 4L); - fail(); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - assertEquals(IllegalArgumentException.class, cause.getClass()); - assertEquals("Error Message 3 4", cause.getMessage()); - } - } - - @Test - public void checkPreconditionsCheckNotNullIntParam() throws Exception { - invokeMethod(Preconditions.class, "checkNotNull", - new Class[] { Object.class, String.class, int.class }, null, this, - "Error Message %s", 5); - try { - invokeMethod(Preconditions.class, "checkNotNull", - new Class[] { Object.class, String.class, int.class }, null, null, - "Error Message %s", 5); - fail(); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - assertEquals(NullPointerException.class, cause.getClass()); - assertEquals("Error Message 5", cause.getMessage()); - } - } - - @Test - public void checkPreconditionsCheckStateIntParam() throws Exception { - invokeMethod(Preconditions.class, "checkState", - new Class[] { boolean.class, String.class, int.class }, null, true, - "Error Message %s", 6); - try { - invokeMethod(Preconditions.class, "checkState", - new Class[] { boolean.class, String.class, int.class }, null, false, - "Error Message %s", 6); - fail(); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - assertEquals(IllegalStateException.class, cause.getClass()); - assertEquals("Error Message 6", cause.getMessage()); - } - } - - @Test - public void checkPreconditionsCheckNotNullVarargs() throws Exception { - checkPreconditionsCheckVarargMethod("checkNotNull", Object.class, this, - null, NullPointerException.class); - } - - @Test - public void checkPreconditionsCheckArgumentVarargs() throws Exception { - checkPreconditionsCheckVarargMethod("checkArgument", boolean.class, true, - false, IllegalArgumentException.class); - } - - @Test - public void checkPreconditionsCheckStateVarargs() throws Exception { - checkPreconditionsCheckVarargMethod("checkState", boolean.class, true, - false, IllegalStateException.class); - } - - private void checkPreconditionsCheckVarargMethod(String methodName, - Class argClass, T goodValue, T badValue, - Class exceptionClass) throws Exception { - for (int i = 1; i < 5; i++) { - String[] templatePlaceholders = new String[i]; - Arrays.fill(templatePlaceholders, "%s"); - - Object[] templateArguments = new Object[i]; - Arrays.setAll(templateArguments, Integer::valueOf); - - String template = "Error message: " - + Stream.of(templatePlaceholders).collect(Collectors.joining(",")); - String message = "Error message: " + Stream.of(templateArguments) - .map(Object::toString).collect(Collectors.joining(",")); - - Class[] parameterTypes = new Class[2 + i]; - parameterTypes[0] = argClass; - parameterTypes[1] = String.class; - Arrays.fill(parameterTypes, 2, parameterTypes.length, Object.class); - - Object[] parameters = new Object[2 + i]; - parameters[0] = goodValue; - parameters[1] = template; - System.arraycopy(templateArguments, 0, parameters, 2, i); - - // Check successful call - invokeMethod(Preconditions.class, methodName, parameterTypes, null, - parameters); - - try { - parameters[0] = badValue; - invokeMethod(Preconditions.class, methodName, parameterTypes, null, - parameters); - fail(); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - assertEquals(exceptionClass, cause.getClass()); - assertEquals(message, cause.getMessage()); - } - } - } - - @SuppressWarnings("unchecked") - private static T invokeMethod(Class clazz, String methodName, - Class[] argumentTypes, Object object, Object... arguments) - throws Exception { - Method method = clazz.getMethod(methodName, argumentTypes); - return (T) method.invoke(object, arguments); - } - -} diff --git a/common/src/test/java/org/apache/drill/test/BaseTest.java b/common/src/test/java/org/apache/drill/test/BaseTest.java index 6256b5e74c9..d247486846a 100644 --- a/common/src/test/java/org/apache/drill/test/BaseTest.java +++ b/common/src/test/java/org/apache/drill/test/BaseTest.java @@ -17,26 +17,9 @@ */ package org.apache.drill.test; -import org.apache.drill.common.util.GuavaPatcher; -import org.apache.drill.common.util.ProtobufPatcher; - /** - * Contains patchers that must be executed at the very beginning of test runs. + * Contains patchers that must be executed at the very beginning of test runs (currently none). * All Drill test classes should be inherited from it to avoid exceptions (e.g. NoSuchMethodError etc.). */ public class BaseTest { - - static { - /* - * HBase and MapR-DB clients use older version of protobuf, - * and override some methods that became final in recent versions. - * This code removes these final modifiers. - */ - ProtobufPatcher.patch(); - /* - * Some libraries, such as Hadoop, HBase, Iceberg depend on incompatible versions of Guava. - * This code adds back some methods to so that the libraries can work with single Guava version. - */ - GuavaPatcher.patch(); - } } diff --git a/contrib/storage-hbase/pom.xml b/contrib/storage-hbase/pom.xml index c2cbc42d2dc..c4decacdea1 100644 --- a/contrib/storage-hbase/pom.xml +++ b/contrib/storage-hbase/pom.xml @@ -39,7 +39,6 @@ org.javassist javassist - test diff --git a/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePlugin.java b/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePlugin.java index f7c15ebde93..a0f1511cb92 100644 --- a/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePlugin.java +++ b/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseStoragePlugin.java @@ -38,6 +38,7 @@ public class HBaseStoragePlugin extends AbstractStoragePlugin { private static final HBaseConnectionManager hbaseConnectionManager = HBaseConnectionManager.INSTANCE; + private final HBaseStoragePluginConfig storeConfig; private final HBaseSchemaFactory schemaFactory; private final HBaseConnectionKey connectionKey; diff --git a/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseUtils.java b/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseUtils.java index 8834b3a35c2..0d861281e52 100644 --- a/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseUtils.java +++ b/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseUtils.java @@ -37,6 +37,15 @@ import com.google.common.collect.Lists; public class HBaseUtils { + static { + /* + * HBase and MapR-DB clients use older version of protobuf, + * and override some methods that became final in recent versions. + * This code removes these final modifiers. + */ + ProtobufPatcher.patch(); + } + static final ParseFilter FILTER_PARSEER = new ParseFilter(); static final int FIRST_FILTER = 0; diff --git a/common/src/main/java/org/apache/drill/common/util/ProtobufPatcher.java b/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/ProtobufPatcher.java similarity index 98% rename from common/src/main/java/org/apache/drill/common/util/ProtobufPatcher.java rename to contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/ProtobufPatcher.java index 4312abcb019..80e9181545b 100644 --- a/common/src/main/java/org/apache/drill/common/util/ProtobufPatcher.java +++ b/contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/ProtobufPatcher.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package org.apache.drill.common.util; +package org.apache.drill.exec.store.hbase; import javassist.CannotCompileException; import javassist.ClassPool; @@ -48,6 +48,7 @@ public static synchronized void patch() { patchByteString(); patchGeneratedMessageLite(); patchGeneratedMessageLiteBuilder(); + logger.info("Patched protobuf for compatibility with HBase client libs."); } } diff --git a/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/DrillApplicationMaster.java b/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/DrillApplicationMaster.java index e144bff48c1..ef93c825dcc 100644 --- a/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/DrillApplicationMaster.java +++ b/drill-yarn/src/main/java/org/apache/drill/yarn/appMaster/DrillApplicationMaster.java @@ -19,8 +19,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.drill.common.util.GuavaPatcher; -import org.apache.drill.common.util.ProtobufPatcher; import org.apache.drill.yarn.appMaster.ControllerFactory.ControllerFactoryException; import org.apache.drill.yarn.appMaster.http.WebServer; import org.apache.drill.yarn.core.DoyConfigException; @@ -47,20 +45,6 @@ public class DrillApplicationMaster { - static { - /* - * Drill-on-YARN uses Hadoop dependencies that use older version of protobuf, - * and override some methods that became final in recent protobuf versions. - * This code removes these final modifiers. - */ - ProtobufPatcher.patch(); - /* - * Some libraries, such as Hadoop or HBase, depend on incompatible versions of Guava. - * This code adds back some methods to so that the libraries can work with single Guava version. - */ - GuavaPatcher.patch(); - } - private static final Log LOG = LogFactory .getLog(DrillApplicationMaster.class); diff --git a/drill-yarn/src/main/java/org/apache/drill/yarn/client/DrillOnYarn.java b/drill-yarn/src/main/java/org/apache/drill/yarn/client/DrillOnYarn.java index 5f79f5da7f1..0225c259a44 100644 --- a/drill-yarn/src/main/java/org/apache/drill/yarn/client/DrillOnYarn.java +++ b/drill-yarn/src/main/java/org/apache/drill/yarn/client/DrillOnYarn.java @@ -17,9 +17,6 @@ */ package org.apache.drill.yarn.client; - -import org.apache.drill.common.util.GuavaPatcher; -import org.apache.drill.common.util.ProtobufPatcher; import org.apache.drill.yarn.core.DoyConfigException; import org.apache.drill.yarn.core.DrillOnYarnConfig; import org.apache.log4j.BasicConfigurator; @@ -76,20 +73,6 @@ public class DrillOnYarn { - static { - /* - * Drill-on-YARN uses Hadoop dependencies that use older version of protobuf, - * and override some methods that became final in recent protobuf versions. - * This code removes these final modifiers. - */ - ProtobufPatcher.patch(); - /* - * Some libraries, such as Hadoop or HBase, depend on incompatible versions of Guava. - * This code adds back some methods to so that the libraries can work with single Guava version. - */ - GuavaPatcher.patch(); - } - public static void main(String argv[]) { BasicConfigurator.configure(); ClientContext.init(); @@ -190,4 +173,4 @@ private static void displayError(CommandLineOptions opts, ClientException e) { e.printStackTrace(ClientContext.err); } } -} \ No newline at end of file +} diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java index cb78340f7e4..ca2547b2de6 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java @@ -47,8 +47,6 @@ import org.apache.drill.exec.store.sys.store.provider.CachingPersistentStoreProvider; import org.apache.drill.exec.store.sys.store.provider.InMemoryStoreProvider; import org.apache.drill.exec.store.sys.store.provider.LocalPersistentStoreProvider; -import org.apache.drill.common.util.GuavaPatcher; -import org.apache.drill.common.util.ProtobufPatcher; import org.apache.drill.exec.work.WorkManager; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; @@ -79,17 +77,6 @@ public class Drillbit implements AutoCloseable { private static final Logger logger = LoggerFactory.getLogger(Drillbit.class); static { - /* - * HBase and MapR-DB clients use older version of protobuf, - * and override some methods that became final in recent versions. - * This code removes these final modifiers. - */ - ProtobufPatcher.patch(); - /* - * Some libraries, such as Hadoop or HBase, depend on incompatible versions of Guava. - * This code adds back some methods to so that the libraries can work with single Guava version. - */ - GuavaPatcher.patch(); Environment.logEnv("Drillbit environment: ", logger); // Jersey uses java.util.logging - create bridge: jul to slf4j SLF4JBridgeHandler.removeHandlersForRootLogger(); diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java index 6b8dd32970d..64b647fe4af 100644 --- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java +++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java @@ -23,8 +23,6 @@ import java.sql.SQLException; import java.util.Properties; -import org.apache.drill.common.util.GuavaPatcher; -import org.apache.drill.common.util.ProtobufPatcher; import org.apache.drill.jdbc.impl.DriverImpl; @@ -43,8 +41,6 @@ public class Driver implements java.sql.Driver { // DriverManager access it: static { - ProtobufPatcher.patch(); - GuavaPatcher.patch(); // Upon loading of class, register an instance with DriverManager. try { DriverManager.registerDriver(new Driver());