From 8ad4bffe0eda133b378dfb7dab0b7acf8b84f3ef Mon Sep 17 00:00:00 2001 From: Arina Ielchiieva Date: Thu, 3 Nov 2016 16:20:04 +0000 Subject: [PATCH] DRILL-4995: Allow lazy init when dynamic UDF support is disabled --- .../fn/FunctionImplementationRegistry.java | 79 +++++++++---------- .../exec/planner/sql/DrillSqlWorker.java | 15 ++-- .../apache/drill/TestDynamicUDFSupport.java | 25 ++++++ .../ExpressionTreeMaterializerTest.java | 10 +++ 4 files changed, 79 insertions(+), 50 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java index 988a9f6b418..d0aa33f12fa 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java @@ -140,9 +140,9 @@ public void register(DrillOperatorTable operatorTable) { } /** - * Using the given functionResolver find Drill function implementation for given - * functionCall - * If function implementation was not found and in case if Dynamic UDF Support is enabled + * Using the given functionResolver + * finds Drill function implementation for given functionCall. + * If function implementation was not found, * loads all missing remote functions and tries to find Drill implementation one more time. */ @Override @@ -154,12 +154,8 @@ private DrillFuncHolder findDrillFunction(FunctionResolver functionResolver, Fun AtomicLong version = new AtomicLong(); DrillFuncHolder holder = functionResolver.getBestMatch( localFunctionRegistry.getMethods(functionReplacement(functionCall), version), functionCall); - if (holder == null && retry) { - if (optionManager != null && optionManager.getOption(ExecConstants.DYNAMIC_UDF_SUPPORT_ENABLED).bool_val) { - if (loadRemoteFunctions(version.get())) { - return findDrillFunction(functionResolver, functionCall, false); - } - } + if (holder == null && retry && loadRemoteFunctions(version.get())) { + return findDrillFunction(functionResolver, functionCall, false); } return holder; } @@ -183,7 +179,7 @@ private String functionReplacement(FunctionCall functionCall) { /** * Find the Drill function implementation that matches the name, arg types and return type. - * If exact function implementation was not found and in case if Dynamic UDF Support is enabled + * If exact function implementation was not found, * loads all missing remote functions and tries to find Drill implementation one more time. */ public DrillFuncHolder findExactMatchingDrillFunction(String name, List argTypes, MajorType returnType) { @@ -197,11 +193,8 @@ private DrillFuncHolder findExactMatchingDrillFunction(String name, List jars = Lists.newArrayList(); - for (String jarName : missingJars) { - Path binary = null; - Path source = null; - URLClassLoader classLoader = null; - try { - binary = copyJarToLocal(jarName, remoteFunctionRegistry); - source = copyJarToLocal(JarUtil.getSourceName(jarName), remoteFunctionRegistry); - URL[] urls = {binary.toUri().toURL(), source.toUri().toURL()}; - classLoader = new URLClassLoader(urls); - ScanResult scanResult = scan(classLoader, binary, urls); - localFunctionRegistry.validate(jarName, scanResult); - jars.add(new JarScan(jarName, scanResult, classLoader)); - } catch (Exception e) { - deleteQuietlyLocalJar(binary); - deleteQuietlyLocalJar(source); - if (classLoader != null) { - try { - classLoader.close(); - } catch (Exception ex) { - logger.warn("Problem during closing class loader for {}", jarName, e); + if (!missingJars.isEmpty()) { + logger.info("Starting dynamic UDFs lazy-init process.\n" + + "The following jars are going to be downloaded and registered locally: " + missingJars); + List jars = Lists.newArrayList(); + for (String jarName : missingJars) { + Path binary = null; + Path source = null; + URLClassLoader classLoader = null; + try { + binary = copyJarToLocal(jarName, remoteFunctionRegistry); + source = copyJarToLocal(JarUtil.getSourceName(jarName), remoteFunctionRegistry); + URL[] urls = {binary.toUri().toURL(), source.toUri().toURL()}; + classLoader = new URLClassLoader(urls); + ScanResult scanResult = scan(classLoader, binary, urls); + localFunctionRegistry.validate(jarName, scanResult); + jars.add(new JarScan(jarName, scanResult, classLoader)); + } catch (Exception e) { + deleteQuietlyLocalJar(binary); + deleteQuietlyLocalJar(source); + if (classLoader != null) { + try { + classLoader.close(); + } catch (Exception ex) { + logger.warn("Problem during closing class loader for {}", jarName, e); + } } + logger.error("Problem during remote functions load from {}", jarName, e); } - logger.error("Problem during remote functions load from {}", jarName, e); } - } - if (!jars.isEmpty()) { - localFunctionRegistry.register(jars); - return true; + if (!jars.isEmpty()) { + localFunctionRegistry.register(jars); + return true; + } } } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java index 19123d0ffde..76529d4a65e 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java @@ -24,7 +24,6 @@ import org.apache.calcite.tools.RelConversionException; import org.apache.calcite.tools.ValidationException; import org.apache.drill.common.exceptions.UserException; -import org.apache.drill.exec.ExecConstants; import org.apache.drill.exec.exception.FunctionNotFoundException; import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry; import org.apache.drill.exec.ops.QueryContext; @@ -113,7 +112,7 @@ public static PhysicalPlan getPlan(QueryContext context, String sql, Pointer)")); + } + + copyDefaultJarsToStagingArea(); + test("create function using jar '%s'", default_binary_name); + + try { + testBuilder() + .sqlQuery("select custom_lower('A') as res from (values(1))") + .optionSettingQueriesForTestQuery("alter system set `exec.udf.enable_dynamic_support` = false") + .unOrdered() + .baselineColumns("res") + .baselineValues("a") + .go(); + } finally { + test("alter system reset `exec.udf.enable_dynamic_support`"); + } + } + + @Test public void testDropFunction() throws Exception { copyDefaultJarsToStagingArea(); diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java index 7d28c9b98e5..8b338af8ce3 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java @@ -21,6 +21,8 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import mockit.Injectable; +import mockit.Mock; +import mockit.MockUp; import mockit.NonStrictExpectations; import org.apache.drill.common.config.DrillConfig; @@ -42,6 +44,8 @@ import org.apache.drill.exec.exception.SchemaChangeException; import org.apache.drill.exec.expr.ExpressionTreeMaterializer; import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry; +import org.apache.drill.exec.expr.fn.registry.RemoteFunctionRegistry; +import org.apache.drill.exec.proto.UserBitShared.Registry; import org.junit.Test; import com.google.common.collect.ImmutableList; @@ -196,6 +200,12 @@ public int getErrorCount() { } }; + new MockUp() { + @Mock + Registry getRegistry() { + return Registry.getDefaultInstance(); + } + }; LogicalExpression functionCallExpr = new FunctionCall("testFunc", ImmutableList.of((LogicalExpression) new FieldReference("test", ExpressionPosition.UNKNOWN) ),