Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/574#discussion_r80558464
  
    --- 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()) {
    --- End diff --
    
    It seems that the version number is always protected by a read or write 
lock. Does it need to be an atomic long? If so, what does the atomic long get 
us that a (lock protected) long does not?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to