drill-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DRILL-4963) Issues when overloading Drill native functions with dynamic UDFs
Date Fri, 24 Feb 2017 12:37:45 GMT

    [ https://issues.apache.org/jira/browse/DRILL-4963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15882583#comment-15882583
] 

ASF GitHub Bot commented on DRILL-4963:
---------------------------------------

Github user arina-ielchiieva commented on a diff in the pull request:

    https://github.com/apache/drill/pull/701#discussion_r102919942
  
    --- 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.
    --- End diff --
    
    At drillbit startup if remote function registry doesn't exist, we put empty remote function
registry in zookeeper with version 0. So we can guarantee that any new dynamically added UDF
will registry version will start with at least 1.


> 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)

Mime
View raw message