Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 2122C200BD5 for ; Thu, 24 Nov 2016 00:42:11 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 1C982160AFD; Wed, 23 Nov 2016 23:42:11 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 24994160B1E for ; Thu, 24 Nov 2016 00:42:09 +0100 (CET) Received: (qmail 84962 invoked by uid 500); 23 Nov 2016 23:42:09 -0000 Mailing-List: contact commits-help@drill.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: commits@drill.apache.org Delivered-To: mailing list commits@drill.apache.org Received: (qmail 84881 invoked by uid 99); 23 Nov 2016 23:42:09 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 23 Nov 2016 23:42:09 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 15546F0DBF; Wed, 23 Nov 2016 23:42:09 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: parthc@apache.org To: commits@drill.apache.org Date: Wed, 23 Nov 2016 23:42:13 -0000 Message-Id: <43feaca34363420bb2f59eab841af8ef@git.apache.org> In-Reply-To: <4d336e529022420ab760f77bea3fe94e@git.apache.org> References: <4d336e529022420ab760f77bea3fe94e@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [5/5] drill git commit: DRILL-4995: Allow lazy init when dynamic UDF support is disabled archived-at: Wed, 23 Nov 2016 23:42:11 -0000 DRILL-4995: Allow lazy init when dynamic UDF support is disabled This closes #645 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/04fb0be1 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/04fb0be1 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/04fb0be1 Branch: refs/heads/master Commit: 04fb0be191ef09409c00ca7173cb903dfbe2abb0 Parents: 3f38118 Author: Arina Ielchiieva Authored: Thu Nov 3 16:20:04 2016 +0000 Committer: Parth Chandra Committed: Wed Nov 23 09:33:30 2016 -0800 ---------------------------------------------------------------------- .../expr/fn/FunctionImplementationRegistry.java | 79 ++++++++++---------- .../drill/exec/planner/sql/DrillSqlWorker.java | 15 ++-- .../org/apache/drill/TestDynamicUDFSupport.java | 25 +++++++ .../record/ExpressionTreeMaterializerTest.java | 10 +++ 4 files changed, 79 insertions(+), 50 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/04fb0be1/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java ---------------------------------------------------------------------- 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 988a9f6..d0aa33f 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 class FunctionImplementationRegistry implements FunctionLookupContext, Au } /** - * 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 @@ public class FunctionImplementationRegistry implements FunctionLookupContext, Au 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 @@ public class FunctionImplementationRegistry implements FunctionLookupContext, Au /** * 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 @@ public class FunctionImplementationRegistry implements FunctionLookupContext, Au return h; } } - - if (retry && optionManager != null && optionManager.getOption(ExecConstants.DYNAMIC_UDF_SUPPORT_ENABLED).bool_val) { - if (loadRemoteFunctions(version.get())) { - return findExactMatchingDrillFunction(name, argTypes, returnType, false); - } + if (retry && loadRemoteFunctions(version.get())) { + return findExactMatchingDrillFunction(name, argTypes, returnType, false); } return null; } @@ -287,35 +280,39 @@ public class FunctionImplementationRegistry implements FunctionLookupContext, Au if (!missingJars.isEmpty()) { synchronized (this) { missingJars = getMissingJars(remoteFunctionRegistry, localFunctionRegistry); - 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; + } } } } http://git-wip-us.apache.org/repos/asf/drill/blob/04fb0be1/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java ---------------------------------------------------------------------- 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 19123d0..76529d4 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.sql.parser.SqlParseException; 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 class DrillSqlWorker { /** * Returns query physical plan. - * In case of {@link FunctionNotFoundException} and dynamic udf support is enabled, attempts to load remote functions. + * In case of {@link FunctionNotFoundException} attempts to load remote functions. * If at least one function was loaded or local function function registry version has changed, * makes one more attempt to get query physical plan. */ @@ -122,13 +121,11 @@ public class DrillSqlWorker { try { return handler.getPlan(sqlNode); } catch (FunctionNotFoundException e) { - if (context.getOption(ExecConstants.DYNAMIC_UDF_SUPPORT_ENABLED).bool_val) { - DrillOperatorTable drillOperatorTable = context.getDrillOperatorTable(); - FunctionImplementationRegistry functionRegistry = context.getFunctionRegistry(); - if (functionRegistry.loadRemoteFunctions(drillOperatorTable.getFunctionRegistryVersion())) { - drillOperatorTable.reloadOperators(functionRegistry); - return handler.getPlan(sqlNode); - } + DrillOperatorTable drillOperatorTable = context.getDrillOperatorTable(); + FunctionImplementationRegistry functionRegistry = context.getFunctionRegistry(); + if (functionRegistry.loadRemoteFunctions(drillOperatorTable.getFunctionRegistryVersion())) { + drillOperatorTable.reloadOperators(functionRegistry); + return handler.getPlan(sqlNode); } throw e; } http://git-wip-us.apache.org/repos/asf/drill/blob/04fb0be1/exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java b/exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java index ae73a3d..4676e39 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestDynamicUDFSupport.java @@ -339,6 +339,31 @@ public class TestDynamicUDFSupport extends BaseTestQuery { } @Test + public void testLazyInitWhenDynamicUdfSupportIsDisabled() throws Exception { + try { + test("select custom_lower('A') from (values(1))"); + } catch (UserRemoteException e){ + assertThat(e.getMessage(), containsString("No match found for function signature custom_lower()")); + } + + 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(); test("create function using jar '%s'", default_binary_name); http://git-wip-us.apache.org/repos/asf/drill/blob/04fb0be1/exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java ---------------------------------------------------------------------- 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 7d28c9b..8b338af 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.assertEquals; 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.ExecTest; 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 class ExpressionTreeMaterializerTest extends ExecTest { } }; + new MockUp() { + @Mock + Registry getRegistry() { + return Registry.getDefaultInstance(); + } + }; LogicalExpression functionCallExpr = new FunctionCall("testFunc", ImmutableList.of((LogicalExpression) new FieldReference("test", ExpressionPosition.UNKNOWN) ),