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=15882584#comment-15882584
] 

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_r102920190
  
    --- 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 --
    
    Agree. Done.


> 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