[
https://issues.apache.org/jira/browse/DRILL-4963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15854510#comment-15854510
]
ASF GitHub Bot commented on DRILL-4963:
---------------------------------------
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/701#discussion_r99649785
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
---
@@ -260,76 +293,101 @@ public RemoteFunctionRegistry
getRemoteFunctionRegistry() {
}
/**
- * Attempts to load and register functions from remote function registry.
- * First checks if there is no missing jars.
- * If yes, enters synchronized block to prevent other loading the same
jars.
- * Again re-checks if there are no missing jars in case someone has
already loaded them (double-check lock).
- * If there are still missing jars, first copies jars to local udf area
and prepares {@link JarScan} for each jar.
- * Jar registration timestamp represented in milliseconds is used as
suffix.
- * Then registers all jars at the same time. Returns true when finished.
- * In case if any errors during jars coping or registration, logs errors
and proceeds.
+ * Purpose of this method is to synchronize remote and local function
registries if needed
+ * and to inform if function registry was changed after given version.
*
- * If no missing jars are found, checks current local registry version.
- * Returns false if versions match, true otherwise.
+ * To make synchronization as much light-weigh as possible, first only
versions of both registries are checked
+ * without any locking. If synchronization is needed, enters
synchronized block to prevent others loading the same jars.
+ * The need of synchronization is checked again (double-check lock)
before comparing jars.
+ * If any missing jars are found, they are downloaded to local udf area,
each is wrapped into {@link JarScan}.
+ * Once jar download is finished, all missing jars are registered in one
batch.
+ * In case if any errors during jars download / registration, these
errors are logged.
*
- * @param version local function registry version
- * @return true if new jars were registered or local function registry
version is different, false otherwise
+ * During registration local function registry is updated with remote
function registry version it is synced with.
+ * When at least one jar of the missing jars failed to download /
register,
+ * local function registry version are not updated but jars that where
successfully downloaded / registered
+ * are added to local function registry.
+ *
+ * If synchronization between remote and local function registry was not
needed,
+ * checks if given registry version matches latest sync version
+ * to inform if function registry was changed after given version.
+ *
+ * @param version remote function registry local function registry was
based on
+ * @return true if remote and local function registries were
synchronized after given version
*/
- public boolean loadRemoteFunctions(long version) {
- List<String> missingJars = getMissingJars(remoteFunctionRegistry,
localFunctionRegistry);
- if (!missingJars.isEmpty()) {
+ public boolean syncWithRemoteRegistry(long version) {
+ if
(doSyncFunctionRegistries(remoteFunctionRegistry.getRegistryVersion(),
localFunctionRegistry.getVersion())) {
synchronized (this) {
- missingJars = getMissingJars(remoteFunctionRegistry,
localFunctionRegistry);
- if (!missingJars.isEmpty()) {
- logger.info("Starting dynamic UDFs lazy-init process.\n" +
- "The following jars are going to be downloaded and
registered locally: " + missingJars);
+ long localRegistryVersion = localFunctionRegistry.getVersion();
+ if
(doSyncFunctionRegistries(remoteFunctionRegistry.getRegistryVersion(),
localRegistryVersion)) {
+ DataChangeVersion remoteVersion = new DataChangeVersion();
+ List<String> missingJars =
getMissingJars(this.remoteFunctionRegistry, localFunctionRegistry,
remoteVersion);
List<JarScan> 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);
+ for (String jarName : missingJars) {
+ Path binary = null;
+ Path source = null;
+ URLClassLoader classLoader = null;
+ try {
+ binary = copyJarToLocal(jarName,
this.remoteFunctionRegistry);
+ source = copyJarToLocal(JarUtil.getSourceName(jarName),
this.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;
- }
+ long latestRegistryVersion = jars.size() != missingJars.size() ?
+ localRegistryVersion : remoteVersion.getVersion();
+ localFunctionRegistry.register(jars, latestRegistryVersion);
+ return true;
}
}
}
+
return version != localFunctionRegistry.getVersion();
}
/**
- * First finds path to marker file url, otherwise throws {@link
JarValidationException}.
- * Then scans jar classes according to list indicated in marker files.
- * Additional logic is added to close {@link URL} after {@link
ConfigFactory#parseURL(URL)}.
- * This is extremely important for Windows users where system doesn't
allow to delete file if it's being used.
+ * Checks if local function registry should be synchronized with remote
function registry.
+ * If remote function registry version is -1, it means that remote
function registry is unreachable
+ * or is not configured thus we skip synchronization and return false.
+ * In all other cases synchronization is needed if remote and local
function registries versions do not match.
*
- * @param classLoader unique class loader for jar
- * @param path local path to jar
- * @param urls urls associated with the jar (ex: binary and source)
- * @return scan result of packages, classes, annotations found in jar
+ * @param remoteVersion remote function registry version
+ * @param localVersion local function registry version
+ * @return true is local registry should be refreshed, false otherwise
*/
+ private boolean doSyncFunctionRegistries(long remoteVersion, long
localVersion) {
--- End diff --
Nit: This is a query, so maybe `isRegistrySyncNeeded`? The "do" prefix is
often used as the implementation, so I expected this to be the implementation
of the registry sync, given the name.
> Issues when overloading Drill native functions with dynamic UDFs
> ----------------------------------------------------------------
>
> Key: DRILL-4963
> URL: https://issues.apache.org/jira/browse/DRILL-4963
> Project: Apache Drill
> Issue Type: Bug
> Components: Functions - Drill
> Affects Versions: 1.9.0
> Reporter: Roman
> Assignee: Arina Ielchiieva
> Fix For: Future
>
> Attachments: subquery_udf-1.0.jar, subquery_udf-1.0-sources.jar,
> test_overloading-1.0.jar, test_overloading-1.0-sources.jar
>
>
> I created jar file which overloads 3 DRILL native functions
> (LOG(VARCHAR-REQUIRED), CURRENT_DATE(VARCHAR-REQUIRED) and
> ABS(VARCHAR-REQUIRED,VARCHAR-REQUIRED)) and registered it as dynamic UDF.
> If I try to use my functions I will get errors:
> {code:xml}
> SELECT CURRENT_DATE('test') FROM (VALUES(1));
> {code}
> Error: FUNCTION ERROR: CURRENT_DATE does not support operand types (CHAR)
> SQL Query null
> {code:xml}
> SELECT ABS('test','test') FROM (VALUES(1));
> {code}
> Error: FUNCTION ERROR: ABS does not support operand types (CHAR,CHAR)
> SQL Query null
> {code:xml}
> SELECT LOG('test') FROM (VALUES(1));
> {code}
> Error: SYSTEM ERROR: DrillRuntimeException: Failure while materializing
> expression in constant expression evaluator LOG('test'). Errors:
> Error in expression at index -1. Error: Missing function implementation:
> castTINYINT(VARCHAR-REQUIRED). Full expression: UNKNOWN EXPRESSION.
> But if I rerun all this queries after "DrillRuntimeException", they will run
> correctly. It seems that Drill have not updated the function signature before
> that error. Also if I add jar as usual UDF (copy jar to
> /drill_home/jars/3rdparty and restart drillbits), all queries will run
> correctly without errors.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
