From b7bc21bb90e68cfd781af4d75fd7130539a82d9a Mon Sep 17 00:00:00 2001 From: Gene Pang Date: Thu, 15 Feb 2018 16:16:04 -0800 Subject: [PATCH] [ALLUXIO-3104] Make factory caching optional (#6881) * [ALLUXIO-3104] Make factory caching optional * [ALLUXIO-3104] Simplify factory registry code * Update javadocs --- .../UnderFileSystemFactoryRegistry.java | 115 +++++++++--------- 1 file changed, 58 insertions(+), 57 deletions(-) diff --git a/core/common/src/main/java/alluxio/underfs/UnderFileSystemFactoryRegistry.java b/core/common/src/main/java/alluxio/underfs/UnderFileSystemFactoryRegistry.java index 1f3776d63544..8326e10e0cd2 100644 --- a/core/common/src/main/java/alluxio/underfs/UnderFileSystemFactoryRegistry.java +++ b/core/common/src/main/java/alluxio/underfs/UnderFileSystemFactoryRegistry.java @@ -31,10 +31,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.ServiceLoader; -import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import javax.annotation.Nullable; @@ -86,9 +84,12 @@ public final class UnderFileSystemFactoryRegistry { private static final Logger LOG = LoggerFactory.getLogger(UnderFileSystemFactoryRegistry.class); - // Key: absolute path to jar file - private static final Set LOADED_LIB_JARS = new HashSet<>(); - private static final Set LOADED_EXTENSION_JARS = new HashSet<>(); + /** + * The base list of factories, which does not include any lib or extension factories. The only + * factories in the base list will be the LocalUFS factory, and any additional factories + * registered by tests. All other UFS factories will be discovered and service loaded when ufs + * creation occurs. + */ private static final List FACTORIES = new CopyOnWriteArrayList<>(); private static boolean sInit = false; @@ -102,9 +103,9 @@ private UnderFileSystemFactoryRegistry() {} } /** - * Returns a read-only view of the available factories. + * Returns a read-only view of the available base factories. * - * @return Read-only view of the available factories + * @return Read-only view of the available base factories */ public static List available() { return Collections.unmodifiableList(FACTORIES); @@ -131,22 +132,15 @@ public static UnderFileSystemFactory find(String path) { @Nullable public static UnderFileSystemFactory find( String path, @Nullable UnderFileSystemConfiguration ufsConf) { - Preconditions.checkArgument(path != null, "path may not be null"); - - scanLibs(); - scanExtensions(); - - for (UnderFileSystemFactory factory : FACTORIES) { - if (factory.supportsPath(path, ufsConf)) { - LOG.debug("Selected Under File System Factory implementation {} for path {}", - factory.getClass(), path); - return factory; - } + List factories = findAll(path, ufsConf); + if (factories.isEmpty()) { + LOG.warn("No Under File System Factory implementation supports the path {}. Please check if " + + "the under storage path is valid.", path); + return null; } - - LOG.warn("No Under File System Factory implementation supports the path {}. Please check if " - + "the under storage path is valid.", path); - return null; + LOG.debug("Selected Under File System Factory implementation {} for path {}", + factories.get(0).getClass(), path); + return factories.get(0); } /** @@ -156,15 +150,16 @@ public static UnderFileSystemFactory find( * @param ufsConf configuration of the UFS * @return list of factories that support the given path which may be an empty list */ - public static List findAll( - String path, UnderFileSystemConfiguration ufsConf) { + public static List findAll(String path, + UnderFileSystemConfiguration ufsConf) { Preconditions.checkArgument(path != null, "path may not be null"); - scanLibs(); - scanExtensions(); + List factories = new ArrayList<>(FACTORIES); + scanLibs(factories); + scanExtensions(factories); List eligibleFactories = new ArrayList<>(); - for (UnderFileSystemFactory factory : FACTORIES) { + for (UnderFileSystemFactory factory : factories) { if (factory.supportsPath(path, ufsConf)) { LOG.debug("Under File System Factory implementation {} is eligible for path {}", factory.getClass(), path); @@ -179,17 +174,21 @@ public static List findAll( } /** - * Finds all {@link UnderFileSystemFactory} from the extensions directory and caches. + * Finds all {@link UnderFileSystemFactory} from the extensions directory. + * + * @param factories list of factories to add to */ - private static void scanExtensions() { + private static void scanExtensions(List factories) { LOG.info("Loading extension UFS jars from {}", Configuration.get(PropertyKey.EXTENSIONS_DIR)); - scan(Arrays.asList(ExtensionUtils.listExtensions()), LOADED_EXTENSION_JARS); + scan(Arrays.asList(ExtensionUtils.listExtensions()), factories); } /** - * Finds all {@link UnderFileSystemFactory} from the lib directory and caches. + * Finds all {@link UnderFileSystemFactory} from the lib directory. + * + * @param factories list of factories to add to */ - private static void scanLibs() { + private static void scanLibs(List factories) { LOG.info("Loading core UFS jars from {}", RuntimeConstants.LIB_DIR); List files = new ArrayList<>(); try (DirectoryStream stream = Files.newDirectoryStream( @@ -202,32 +201,29 @@ private static void scanLibs() { } catch (IOException e) { LOG.warn("Failed to load UFS libs: {}", e.getMessage()); } - scan(files, LOADED_LIB_JARS); + scan(files, factories); } /** * Class-loads jar files that have not been loaded. * * @param files jar files to class-load - * @param loadedJars jars already loaded under this dir + * @param factories list of factories to add to */ - private static void scan(List files, Set loadedJars) { + private static void scan(List files, List factories) { for (File jar : files) { try { URL extensionURL = jar.toURI().toURL(); String jarPath = extensionURL.toString(); - if (!loadedJars.contains(jarPath)) { - ClassLoader extensionsClassLoader = new ExtensionsClassLoader(new URL[] {extensionURL}, - ClassLoader.getSystemClassLoader()); - ServiceLoader extensionServiceLoader = - ServiceLoader.load(UnderFileSystemFactory.class, extensionsClassLoader); - for (UnderFileSystemFactory factory : extensionServiceLoader) { - LOG.debug("Discovered an Under File System Factory implementation {} - {} in jar {}", - factory.getClass(), factory.toString(), jarPath); - // Cache - register(factory); - loadedJars.add(jarPath); - } + + ClassLoader extensionsClassLoader = new ExtensionsClassLoader(new URL[] {extensionURL}, + ClassLoader.getSystemClassLoader()); + ServiceLoader extensionServiceLoader = + ServiceLoader.load(UnderFileSystemFactory.class, extensionsClassLoader); + for (UnderFileSystemFactory factory : extensionServiceLoader) { + LOG.debug("Discovered an Under File System Factory implementation {} - {} in jar {}", + factory.getClass(), factory.toString(), jarPath); + register(factory, factories); } } catch (Throwable t) { LOG.warn("Failed to load jar {}: {}", jar, t.getMessage()); @@ -240,19 +236,16 @@ private static synchronized void init() { return; } - // Discover and register the available factories + // Discover and register the available base factories ServiceLoader discoveredFactories = ServiceLoader.load(UnderFileSystemFactory.class, UnderFileSystemFactory.class.getClassLoader()); for (UnderFileSystemFactory factory : discoveredFactories) { - LOG.debug("Discovered Under File System Factory implementation {} - {}", factory.getClass(), - factory.toString()); + LOG.debug("Discovered base Under File System Factory implementation {} - {}", + factory.getClass(), factory.toString()); register(factory); } - scanLibs(); - scanExtensions(); - sInit = true; } @@ -269,6 +262,11 @@ private static synchronized void init() { * @param factory factory to register */ public static void register(UnderFileSystemFactory factory) { + register(factory, FACTORIES); + } + + private static void register(UnderFileSystemFactory factory, + List factories) { if (factory == null) { return; } @@ -278,7 +276,7 @@ public static void register(UnderFileSystemFactory factory) { // Insert at start of list so it will take precedence over automatically discovered and // previously registered factories - FACTORIES.add(0, factory); + factories.add(0, factory); } /** @@ -292,8 +290,6 @@ public static synchronized void reset() { // Reset state sInit = false; FACTORIES.clear(); - LOADED_LIB_JARS.clear(); - LOADED_EXTENSION_JARS.clear(); } // Reinitialise @@ -306,12 +302,17 @@ public static synchronized void reset() { * @param factory factory to unregister */ public static void unregister(UnderFileSystemFactory factory) { + unregister(factory, FACTORIES); + } + + private static void unregister(UnderFileSystemFactory factory, + List factories) { if (factory == null) { return; } LOG.debug("Unregistered Under File System Factory implementation {} - {}", factory.getClass(), factory.toString()); - FACTORIES.remove(factory); + factories.remove(factory); } }