Add ExtensionsConfig.excludeModules (#4438)

* Add ExtensionsConfig.excludeModules

* Add branch

* Refactor Initialization.getFromExtensions()

* excludeModules -> moduleExcludeList

* Initialization.getFromExtensions() and getLoadedModules() should return Collection, not Set

* Fix doc

docs/content/configuration/index.md

@@ -24,6 +24,7 @@ Many of Druid's external dependencies can be plugged in as modules. Extensions c
 |`druid.extensions.directory`|The root extension directory where user can put extensions related files. Druid will load extensions stored under this directory.|`extensions` (This is a relative path to Druid's working directory)|
 |`druid.extensions.hadoopDependenciesDir`|The root hadoop dependencies directory where user can put hadoop related dependencies files. Druid will load the dependencies based on the hadoop coordinate specified in the hadoop index task.|`hadoop-dependencies` (This is a relative path to Druid's working directory|
 |`druid.extensions.loadList`|A JSON array of extensions to load from extension directories by Druid. If it is not specified, its value will be `null` and Druid will load all the extensions under `druid.extensions.directory`. If its value is empty list `[]`, then no extensions will be loaded at all. It is also allowed to specify absolute path of other custom extensions not stored in the common extensions directory.|null|
+|`druid.extensions.moduleExcludeList`|A JSON array of canonical class names (e. g. `"io.druid.somepackage.SomeModule"`) of module classes which shouldn't be loaded, even if they are found in extensions specified by `druid.extensions.loadList`. Useful when some useful extension contains some module, which shouldn't be loaded on some Druid node type because some dependencies of that module couldn't be satisfied.|[]|
 |`druid.extensions.searchCurrentClassloader`|This is a boolean flag that determines if Druid will search the main classloader for extensions.  It defaults to true but can be turned off if you have reason to not automatically add all modules on the classpath.|true|
 |`druid.extensions.hadoopContainerDruidClasspath`|Hadoop Indexing launches hadoop jobs and this configuration provides way to explicitly set the user classpath for the hadoop job. By default this is computed automatically by druid based on the druid process classpath and set of extensions. However, sometimes you might want to be explicit to resolve dependency conflicts between druid and hadoop.|null|
 |`druid.extensions.addExtensionsToHadoopContainer`|Only applicable if `druid.extensions.hadoopContainerDruidClasspath` is provided. If set to true, then extensions specified in the loadList are added to hadoop container classpath. Note that when `druid.extensions.hadoopContainerDruidClasspath` is not provided then extensions are always added to hadoop container classpath.|false|

processing/src/main/java/io/druid/guice/ExtensionsConfig.java

@@ -22,6 +22,7 @@ package io.druid.guice;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 import javax.validation.constraints.NotNull;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -48,6 +49,13 @@ public class ExtensionsConfig
   @JsonProperty
   private List<String> loadList;
 
+  /**
+   * Canonical class names of modules, which should not be loaded despite they are founded in extensions from {@link
+   * #loadList}.
+   */
+  @JsonProperty
+  private List<String> moduleExcludeList = Collections.emptyList();
+
   public boolean searchCurrentClassloader()
   {
     return searchCurrentClassloader;
@@ -78,6 +86,11 @@ public class ExtensionsConfig
     return loadList;
   }
 
+  public List<String> getModuleExcludeList()
+  {
+    return moduleExcludeList;
+  }
+
   @Override
   public String toString()
   {
@@ -88,6 +101,7 @@ public class ExtensionsConfig
            ", hadoopContainerDruidClasspath='" + hadoopContainerDruidClasspath + '\'' +
            ", addExtensionsToHadoopContainer=" + addExtensionsToHadoopContainer +
            ", loadList=" + loadList +
+           ", moduleExcludeList=" + moduleExcludeList +
            '}';
   }
 }

server/src/main/java/io/druid/initialization/Initialization.java

@@ -74,6 +74,7 @@ import java.net.URLClassLoader;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.ServiceLoader;
@@ -88,17 +89,18 @@ public class Initialization
   private static final Logger log = new Logger(Initialization.class);
   private static final ConcurrentMap<File, URLClassLoader> loadersMap = new ConcurrentHashMap<>();
 
-  private final static Map<Class, Set> extensionsMap = Maps.<Class, Set>newHashMap();
+  private final static Map<Class, Collection> extensionsMap = Maps.newHashMap();
 
   /**
-   * @param clazz Module class
-   * @param <T>
+   * @param clazz service class
+   * @param <T> the service type
    *
-   * @return Returns the set of modules loaded.
+   * @return Returns a collection of implementations loaded.
    */
-  public static <T> Set<T> getLoadedModules(Class<T> clazz)
+  public static <T> Collection<T> getLoadedImplementations(Class<T> clazz)
   {
-    Set<T> retVal = extensionsMap.get(clazz);
+    @SuppressWarnings("unchecked")
+    Collection<T> retVal = extensionsMap.get(clazz);
     if (retVal == null) {
       return Sets.newHashSet();
     }
@@ -106,7 +108,7 @@ public class Initialization
   }
 
   @VisibleForTesting
-  static void clearLoadedModules()
+  static void clearLoadedImplementations()
   {
     extensionsMap.clear();
   }
@@ -118,63 +120,84 @@ public class Initialization
   }
 
   /**
-   * Look for extension modules for the given class from both classpath and extensions directory. A user should never
-   * put the same two extensions in classpath and extensions directory, if he/she does that, the one that is in the
-   * classpath will be loaded, the other will be ignored.
+   * Look for implementations for the given class from both classpath and extensions directory, using {@link
+   * java.util.ServiceLoader}. A user should never put the same two extensions in classpath and extensions directory, if
+   * he/she does that, the one that is in the classpath will be loaded, the other will be ignored.
    *
-   * @param config Extensions configuration
-   * @param clazz  The class of extension module (e.g., DruidModule)
+   * @param config        Extensions configuration
+   * @param serviceClass  The class to look the implementations of (e.g., DruidModule)
    *
-   * @return A collection that contains distinct extension modules
+   * @return A collection that contains implementations (of distinct concrete classes) of the given class. The order of
+   * elements in the returned collection is not specified and not guaranteed to be the same for different calls to
+   * getFromExtensions().
    */
-  public synchronized static <T> Collection<T> getFromExtensions(ExtensionsConfig config, Class<T> clazz)
+  public synchronized static <T> Collection<T> getFromExtensions(ExtensionsConfig config, Class<T> serviceClass)
   {
-    final Set<T> retVal = Sets.newHashSet();
-    final Set<String> loadedExtensionNames = Sets.newHashSet();
+    Collection<T> modulesToLoad = new ServiceLoadingFromExtensions<>(config, serviceClass).implsToLoad;
+    extensionsMap.put(serviceClass, modulesToLoad);
+    return modulesToLoad;
+  }
 
-    if (config.searchCurrentClassloader()) {
-      for (T module : ServiceLoader.load(clazz, Thread.currentThread().getContextClassLoader())) {
-        final String moduleName = module.getClass().getCanonicalName();
-        if (moduleName == null) {
-          log.warn(
-              "Extension module [%s] was ignored because it doesn't have a canonical name, is it a local or anonymous class?",
-              module.getClass().getName()
-          );
-        } else if (!loadedExtensionNames.contains(moduleName)) {
-          log.info("Adding classpath extension module [%s] for class [%s]", moduleName, clazz.getName());
-          loadedExtensionNames.add(moduleName);
-          retVal.add(module);
+  private static class ServiceLoadingFromExtensions<T>
+  {
+    private final ExtensionsConfig extensionsConfig;
+    private final Class<T> serviceClass;
+    private final List<T> implsToLoad = new ArrayList<>();
+    private final Set<String> implClassNamesToLoad = new HashSet<>();
+
+    private ServiceLoadingFromExtensions(ExtensionsConfig extensionsConfig, Class<T> serviceClass)
+    {
+      this.extensionsConfig = extensionsConfig;
+      this.serviceClass = serviceClass;
+      if (extensionsConfig.searchCurrentClassloader()) {
+        addAllFromCurrentClassLoader();
+      }
+      addAllFromFileSystem();
+    }
+
+    private void addAllFromCurrentClassLoader()
+    {
+      ServiceLoader
+          .load(serviceClass, Thread.currentThread().getContextClassLoader())
+          .forEach(impl -> tryAdd(impl, "classpath"));
+    }
+
+    private void addAllFromFileSystem()
+    {
+      for (File extension : getExtensionFilesToLoad(extensionsConfig)) {
+        log.info("Loading extension [%s] for class [%s]", extension.getName(), serviceClass);
+        try {
+          final URLClassLoader loader = getClassLoaderForExtension(extension);
+          ServiceLoader.load(serviceClass, loader).forEach(impl -> tryAdd(impl, "local file system"));
+        }
+        catch (Exception e) {
+          throw Throwables.propagate(e);
         }
       }
     }
 
-    for (File extension : getExtensionFilesToLoad(config)) {
-      log.info("Loading extension [%s] for class [%s]", extension.getName(), clazz.getName());
-      try {
-        final URLClassLoader loader = getClassLoaderForExtension(extension);
-        for (T module : ServiceLoader.load(clazz, loader)) {
-          final String moduleName = module.getClass().getCanonicalName();
-          if (moduleName == null) {
-            log.warn(
-                "Extension module [%s] was ignored because it doesn't have a canonical name, is it a local or anonymous class?",
-                module.getClass().getName()
-            );
-          } else if (!loadedExtensionNames.contains(moduleName)) {
-            log.info("Adding local file system extension module [%s] for class [%s]", moduleName, clazz.getName());
-            loadedExtensionNames.add(moduleName);
-            retVal.add(module);
-          }
-        }
-      }
-      catch (Exception e) {
-        throw Throwables.propagate(e);
+    private void tryAdd(T serviceImpl, String extensionType)
+    {
+      final String serviceImplName = serviceImpl.getClass().getCanonicalName();
+      if (serviceImplName == null) {
+        log.warn(
+            "Implementation [%s] was ignored because it doesn't have a canonical name, "
+            + "is it a local or anonymous class?",
+            serviceImpl.getClass().getName()
+        );
+      } else if (extensionsConfig.getModuleExcludeList().contains(serviceImplName)) {
+        log.info("Not loading module [%s] because it is present in moduleExcludeList", serviceImplName);
+      } else if (!implClassNamesToLoad.contains(serviceImplName)) {
+        log.info(
+            "Adding implementation [%s] for class [%s] from %s extension",
+            serviceImplName,
+            serviceClass,
+            extensionType
+        );
+        implClassNamesToLoad.add(serviceImplName);
+        implsToLoad.add(serviceImpl);
       }
     }
-
-    // update the map with currently loaded modules
-    extensionsMap.put(clazz, retVal);
-
-    return retVal;
   }
 
   /**

server/src/main/java/io/druid/server/StatusResource.java

@@ -44,7 +44,7 @@ public class StatusResource
   @Produces(MediaType.APPLICATION_JSON)
   public Status doGet()
   {
-    return new Status(Initialization.getLoadedModules(DruidModule.class));
+    return new Status(Initialization.getLoadedImplementations(DruidModule.class));
   }
 
   public static class Status

server/src/test/java/io/druid/initialization/InitializationTest.java

@@ -28,7 +28,6 @@ import com.google.common.collect.Sets;
 import com.google.inject.Binder;
 import com.google.inject.Injector;
 import com.google.inject.Key;
-
 import io.druid.guice.ExtensionsConfig;
 import io.druid.guice.GuiceInjectors;
 import io.druid.guice.JsonConfigProvider;
@@ -50,6 +49,7 @@ import java.net.URLClassLoader;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -62,11 +62,11 @@ public class InitializationTest
   @Test
   public void test01InitialModulesEmpty() throws Exception
   {
-    Initialization.clearLoadedModules();
+    Initialization.clearLoadedImplementations();
     Assert.assertEquals(
         "Initial set of loaded modules must be empty",
         0,
-        Initialization.getLoadedModules(DruidModule.class).size()
+        Initialization.getLoadedImplementations(DruidModule.class).size()
     );
   }
 
@@ -95,7 +95,7 @@ public class InitializationTest
 
     Assert.assertFalse(
         "modules does not contain TestDruidModule",
-        Collections2.transform(Initialization.getLoadedModules(DruidModule.class), fnClassName)
+        Collections2.transform(Initialization.getLoadedImplementations(DruidModule.class), fnClassName)
                     .contains("io.druid.initialization.InitializationTest.TestDruidModule")
     );
 
@@ -179,13 +179,16 @@ public class InitializationTest
   public void testGetLoadedModules()
   {
 
-    Set<DruidModule> modules = Initialization.getLoadedModules(DruidModule.class);
+    Collection<DruidModule> modules = Initialization.getLoadedImplementations(DruidModule.class);
+    HashSet<DruidModule> moduleSet = new HashSet<>(modules);
 
-    Set<DruidModule> loadedModules = Initialization.getLoadedModules(DruidModule.class);
-    Assert.assertEquals("Set from loaded modules #1 should be same!", modules, loadedModules);
+    Collection<DruidModule> loadedModules = Initialization.getLoadedImplementations(DruidModule.class);
+    Assert.assertEquals("Set from loaded modules #1 should be same!", modules.size(), loadedModules.size());
+    Assert.assertEquals("Set from loaded modules #1 should be same!", moduleSet, new HashSet<>(loadedModules));
 
-    Set<DruidModule> loadedModules2 = Initialization.getLoadedModules(DruidModule.class);
-    Assert.assertEquals("Set from loaded modules #2 should be same!", modules, loadedModules2);
+    Collection<DruidModule> loadedModules2 = Initialization.getLoadedImplementations(DruidModule.class);
+    Assert.assertEquals("Set from loaded modules #2 should be same!", modules.size(), loadedModules2.size());
+    Assert.assertEquals("Set from loaded modules #2 should be same!", moduleSet, new HashSet<>(loadedModules2));
   }
 
   @Test

services/src/main/java/io/druid/cli/Version.java

@@ -33,6 +33,6 @@ public class Version implements Runnable
   @Override
   public void run()
   {
-    System.out.println(new StatusResource.Status(Initialization.getLoadedModules(DruidModule.class)));
+    System.out.println(new StatusResource.Status(Initialization.getLoadedImplementations(DruidModule.class)));
   }
 }