[ https://issues.apache.org/jira/browse/DRILL-4726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15524287#comment-15524287 ]
ASF GitHub Bot commented on DRILL-4726: --------------------------------------- Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/574#discussion_r80557682 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/registry/FunctionRegistryHolder.java --- @@ -0,0 +1,360 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.expr.fn.registry; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Queues; +import org.apache.drill.common.concurrent.AutoCloseableLock; +import org.apache.drill.exec.expr.fn.DrillFuncHolder; + +import java.util.List; +import java.util.Map; +import java.util.Queue; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + +/** + * Function registry holder stores function implementations by jar name, function name. + * Contains two maps that hold data by jars and functions respectively. + * Jars map contains each jar as a key and map of all its functions with collection of function signatures as value. + * Functions map contains function name as key and map of its signatures and function holder as value. + * All maps and collections used are concurrent to guarantee memory consistency effects. + * Such structure is chosen to achieve maximum speed while retrieving data by jar or by function name, + * since we expect infrequent registry changes. + * Holder is designed to allow concurrent reads and single writes to keep data consistent. + * This is achieved by {@link ReadWriteLock} implementation usage. + * Holder has number version which changes every time new jars are added or removed. Initial version number is 0. + * Also version is used when user needs data from registry with version it is based on. + * + * Structure example: + * + * JARS + * built-in -> upper -> upper(VARCHAR-REQUIRED) + * -> lower -> lower(VARCHAR-REQUIRED) + * + * First.jar -> upper -> upper(VARCHAR-OPTIONAL) + * -> custom_upper -> custom_upper(VARCHAR-REQUIRED) + * -> custom_upper(VARCHAR-OPTIONAL) + * + * Second.jar -> lower -> lower(VARCHAR-OPTIONAL) + * -> custom_upper -> custom_upper(VARCHAR-REQUIRED) + * -> custom_upper(VARCHAR-OPTIONAL) + * + * FUNCTIONS + * upper -> upper(VARCHAR-REQUIRED) -> function holder for upper(VARCHAR-REQUIRED) + * -> upper(VARCHAR-OPTIONAL) -> function holder for upper(VARCHAR-OPTIONAL) + * + * lower -> lower(VARCHAR-REQUIRED) -> function holder for lower(VARCHAR-REQUIRED) + * -> lower(VARCHAR-OPTIONAL) -> function holder for lower(VARCHAR-OPTIONAL) + * + * custom_upper -> custom_upper(VARCHAR-REQUIRED) -> function holder for custom_upper(VARCHAR-REQUIRED) + * -> custom_upper(VARCHAR-OPTIONAL) -> function holder for custom_upper(VARCHAR-OPTIONAL) + * + * custom_lower -> custom_lower(VARCHAR-REQUIRED) -> function holder for custom_lower(VARCHAR-REQUIRED) + * -> custom_lower(VARCHAR-OPTIONAL) -> function holder for custom_lower(VARCHAR-OPTIONAL) + * + * where + * First.jar is jar name represented by String + * upper is function name represented by String + * upper(VARCHAR-REQUIRED) is signature name represented by String which consist of function name, list of input parameters + * function holder for upper(VARCHAR-REQUIRED) is {@link DrillFuncHolder} initiated for each function. + * + */ +public class FunctionRegistryHolder { + + private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); + private final AutoCloseableLock readLock = new AutoCloseableLock(readWriteLock.readLock()); + private final AutoCloseableLock writeLock = new AutoCloseableLock(readWriteLock.writeLock()); + private final AtomicLong version = new AtomicLong(); + + // jar name, Map<function name, Queue<function signature> + private final Map<String, Map<String, Queue<String>>> jars; + + // function name, Map<function signature, function holder> + private final Map<String, Map<String, DrillFuncHolder>> functions; + + public FunctionRegistryHolder() { + this.functions = Maps.newConcurrentMap(); + this.jars = Maps.newConcurrentMap(); + } + + /** + * This is read operation, so several users at a time can get this data. + * @return local function registry version number + */ + public long getVersion() { + try (AutoCloseableLock lock = readLock.open()) { + return version.get(); + } + } + + /** + * Adds jars to the function registry. + * If jar with the same name already exists, it and its functions will be removed. + * Then jar will be added to {@link #jars} + * and each function will be added using {@link #addFunctions(Map, List)}. + * Function version registry will be incremented by 1 if at least one jar was added but not for each jar. + * This is write operation, so one user at a time can call perform such action, + * others will wait till first user completes his action. + * + * @param newJars jars and list of their function holders, each contains function name, signature and holder + */ + public void addJars(Map<String, List<FunctionHolder>> newJars) { + try (AutoCloseableLock lock = writeLock.open()) { + for (Map.Entry<String, List<FunctionHolder>> newJar : newJars.entrySet()) { + String jarName = newJar.getKey(); + removeAllByJar(jarName); + Map<String, Queue<String>> jar = Maps.newConcurrentMap(); --- End diff -- No harm in using a concurrent map. But, if the only changes to the map occur within a write lock, is a concurrent map necessary? Or, is there a path to change the map outside the lock? > Dynamic UDFs support > -------------------- > > Key: DRILL-4726 > URL: https://issues.apache.org/jira/browse/DRILL-4726 > Project: Apache Drill > Issue Type: New Feature > Affects Versions: 1.6.0 > Reporter: Arina Ielchiieva > Assignee: Arina Ielchiieva > Fix For: Future > > > Allow register UDFs without restart of Drillbits. > Design is described in document below: > https://docs.google.com/document/d/1FfyJtWae5TLuyheHCfldYUpCdeIezR2RlNsrOTYyAB4/edit?usp=sharing > -- This message was sent by Atlassian JIRA (v6.3.4#6332)