Skip to content
Permalink
Browse files

Fix UfsConfiguration after singleton changes

Remove redundant configuration objects since ufs conf contains a copy of
global configuration

Changes:
- ufs conf is no longer Nullable
- remove duplication alluxio conf from public ufs api

pr-link: #9058
change-id: cid-5054e4ecc374e319fea1fef68b3e3de27ad19325
  • Loading branch information...
madanadit authored and alluxio-bot committed May 14, 2019
1 parent 797ddb9 commit d95cdea120d8bbabcdfd9d19ebec7aff38f0d0b7
Showing with 231 additions and 303 deletions.
  1. +6 −9 core/common/src/main/java/alluxio/extensions/ExtensionFactory.java
  2. +5 −6 core/common/src/main/java/alluxio/extensions/ExtensionFactoryRegistry.java
  3. +1 −7 core/common/src/main/java/alluxio/underfs/BaseUnderFileSystem.java
  4. +2 −5 core/common/src/main/java/alluxio/underfs/ConsistentUnderFileSystem.java
  5. +5 −6 core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java
  6. +7 −21 core/common/src/main/java/alluxio/underfs/UnderFileSystem.java
  7. +0 −8 core/common/src/main/java/alluxio/underfs/UnderFileSystemConfiguration.java
  8. +6 −11 core/common/src/main/java/alluxio/underfs/UnderFileSystemFactory.java
  9. +12 −13 core/common/src/main/java/alluxio/underfs/UnderFileSystemFactoryRegistry.java
  10. +6 −7 core/common/src/main/java/alluxio/underfs/UnderFileSystemWithLogging.java
  11. +6 −1 core/common/src/test/java/alluxio/UnderFileSystemFactoryRegistryRuleTest.java
  12. +2 −1 core/common/src/test/java/alluxio/underfs/AbstractUnderFileSystemContractTest.java
  13. +4 −3 core/common/src/test/java/alluxio/underfs/UnderFileSystemConfigurationTest.java
  14. +3 −1 core/server/common/src/main/java/alluxio/master/journal/JournalUpgrader.java
  15. +3 −1 core/server/common/src/main/java/alluxio/master/journal/ufs/UfsJournalFileParser.java
  16. +3 −1 core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsJournal.java
  17. +3 −1 core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsJournalReader.java
  18. +3 −1 core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsJournalWriter.java
  19. +3 −1 core/server/common/src/main/java/alluxio/master/journalv0/ufs/UfsMutableJournal.java
  20. +2 −2 core/server/common/src/main/java/alluxio/underfs/AbstractUfsManager.java
  21. +3 −2 core/server/master/src/main/java/alluxio/master/meta/DefaultMetaMaster.java
  22. +5 −4 core/server/master/src/test/java/alluxio/master/file/PersistenceTest.java
  23. +5 −4 core/server/master/src/test/java/alluxio/master/file/meta/AsyncUfsAbsentPathCacheTest.java
  24. +4 −3 core/server/master/src/test/java/alluxio/master/file/meta/LazyUfsBlockLocationCacheTest.java
  25. +2 −3 core/server/master/src/test/java/alluxio/master/file/meta/MountTableTest.java
  26. +2 −1 core/server/master/src/test/java/alluxio/master/journal/ufs/UfsJournalCheckpointThreadTest.java
  27. +2 −1 core/server/master/src/test/java/alluxio/master/journal/ufs/UfsJournalCheckpointWriterTest.java
  28. +2 −1 core/server/master/src/test/java/alluxio/master/journal/ufs/UfsJournalLogWriterTest.java
  29. +2 −1 core/server/master/src/test/java/alluxio/master/journal/ufs/UfsJournalReaderTest.java
  30. +1 −5 core/server/master/src/test/java/alluxio/master/meta/AlluxioMasterRestServiceHandlerTest.java
  31. +3 −1 core/server/worker/src/test/java/alluxio/worker/block/UnderFileSystemBlockReaderTest.java
  32. +1 −3 tests/src/test/java/alluxio/client/fs/FileSystemMasterIntegrationTest.java
  33. +1 −1 tests/src/test/java/alluxio/client/fs/MultiUfsMountIntegrationTest.java
  34. +4 −4 tests/src/test/java/alluxio/server/ft/journal/JournalShutdownIntegrationTest.java
  35. +5 −4 tests/src/test/java/alluxio/server/ft/journal/ufs/UfsJournalIntegrationTest.java
  36. +2 −5 tests/src/test/java/alluxio/testutils/underfs/ConfExpectingUnderFileSystemFactory.java
  37. +1 −5 tests/src/test/java/alluxio/testutils/underfs/delegating/DelegatingUnderFileSystemFactory.java
  38. +2 −3 tests/src/test/java/alluxio/testutils/underfs/sleeping/SleepingUnderFileSystem.java
  39. +2 −4 tests/src/test/java/alluxio/testutils/underfs/sleeping/SleepingUnderFileSystemFactory.java
  40. +7 −10 underfs/cos/src/main/java/alluxio/underfs/cos/COSUnderFileSystem.java
  41. +2 −4 underfs/cos/src/main/java/alluxio/underfs/cos/COSUnderFileSystemFactory.java
  42. +6 −11 underfs/gcs/src/main/java/alluxio/underfs/gcs/GCSUnderFileSystem.java
  43. +2 −4 underfs/gcs/src/main/java/alluxio/underfs/gcs/GCSUnderFileSystemFactory.java
  44. +2 −2 underfs/gcs/src/test/java/alluxio/underfs/gcs/GCSUnderFileSystemTest.java
  45. +5 −9 underfs/hdfs/src/main/java/alluxio/underfs/hdfs/HdfsUnderFileSystem.java
  46. +3 −4 underfs/hdfs/src/main/java/alluxio/underfs/hdfs/HdfsUnderFileSystemFactory.java
  47. +7 −5 underfs/hdfs/src/test/java/alluxio/underfs/hdfs/HdfsUnderFileSystemTest.java
  48. +8 −9 underfs/kodo/src/main/java/alluxio/underfs/kodo/KodoUnderFileSystem.java
  49. +3 −7 underfs/kodo/src/main/java/alluxio/underfs/kodo/KodoUnderFileSystemFactory.java
  50. +1 −1 underfs/kodo/src/test/java/alluxio/underfs/kodo/KodoUnderFileSystemTest.java
  51. +5 −8 underfs/local/src/main/java/alluxio/underfs/local/LocalUnderFileSystem.java
  52. +2 −4 underfs/local/src/main/java/alluxio/underfs/local/LocalUnderFileSystemFactory.java
  53. +3 −1 underfs/local/src/test/java/alluxio/underfs/local/LocalUnderFileSystemTest.java
  54. +8 −9 underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java
  55. +2 −4 underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystemFactory.java
  56. +2 −4 underfs/oss/src/test/java/alluxio/underfs/oss/OSSUnderFileSystemTest.java
  57. +15 −17 underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java
  58. +2 −4 underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystemFactory.java
  59. +1 −1 underfs/s3a/src/test/java/alluxio/underfs/s3a/S3AUnderFileSystemTest.java
  60. +5 −8 underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystem.java
  61. +2 −4 underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystemFactory.java
  62. +5 −8 underfs/wasb/src/main/java/alluxio/underfs/wasb/WasbUnderFileSystem.java
  63. +2 −4 underfs/wasb/src/main/java/alluxio/underfs/wasb/WasbUnderFileSystemFactory.java
@@ -13,33 +13,30 @@

import alluxio.conf.AlluxioConfiguration;

import javax.annotation.Nullable;

/**
* A factory class for creating instance of {@link T} based on configuration {@link S}.
* @param <T> The type of instance to be created
* @param <S> the type of configuration to be used when creating the extension
*/
public interface ExtensionFactory<T, S> {
public interface ExtensionFactory<T, S extends AlluxioConfiguration> {
/**
* Creates a new extension for the given path. An {@link IllegalArgumentException} is
* thrown if this factory does not support extension for the given path or if the configuration
* provided is insufficient to create an extension.
*
* @param path file path with scheme for which the extension will be created
* @param conf optional configuration object for the extension, may be null
* @param alluxioConf Alluxio configuration object
* @param conf configuration object for the extension
* @return the new extension
*/
T create(String path, @Nullable S conf, AlluxioConfiguration alluxioConf);
T create(String path, S conf);

/**
* Gets whether this factory supports the given path and thus whether calling the
* {@link #create(String, S, AlluxioConfiguration)} can succeed for this path.
* {@link #create(String, S)} can succeed for this path.
*
* @param path file path with scheme for which the extension will be created
* @param conf optional configuration object for the extension, may be null
* @param conf configuration object for the extension
* @return true if the path is supported, false otherwise
*/
boolean supportsPath(String path, @Nullable S conf);
boolean supportsPath(String path, S conf);
}
@@ -34,7 +34,6 @@
import java.util.ServiceLoader;
import java.util.concurrent.CopyOnWriteArrayList;

import javax.annotation.Nullable;
import javax.annotation.concurrent.NotThreadSafe;

/**
@@ -75,7 +74,8 @@
* @param <S> the type of configuration to be used when creating the extension
*/
@NotThreadSafe
public class ExtensionFactoryRegistry<T extends ExtensionFactory<?, S>, S> {
public class ExtensionFactoryRegistry<T extends ExtensionFactory<?, S>,
S extends AlluxioConfiguration> {
private static final Logger LOG = LoggerFactory.getLogger(ExtensionFactoryRegistry.class);

/**
@@ -125,15 +125,14 @@ private synchronized void init() {
*
* @param path path
* @param conf configuration of the extension
* @param alluxioConf Alluxio configuration
* @return list of factories that support the given path which may be an empty list
*/
public List<T> findAll(String path, @Nullable S conf, AlluxioConfiguration alluxioConf) {
public List<T> findAll(String path, S conf) {
Preconditions.checkArgument(path != null, "path may not be null");

List<T> factories = new ArrayList<>(mFactories);
String libDir = PathUtils.concatPath(alluxioConf.get(PropertyKey.HOME), "lib");
String extensionDir = alluxioConf.get(PropertyKey.EXTENSIONS_DIR);
String libDir = PathUtils.concatPath(conf.get(PropertyKey.HOME), "lib");
String extensionDir = conf.get(PropertyKey.EXTENSIONS_DIR);
scanLibs(factories, libDir);
scanExtensions(factories, extensionDir);

@@ -15,7 +15,6 @@
import alluxio.Constants;
import alluxio.SyncInfo;
import alluxio.collections.Pair;
import alluxio.conf.AlluxioConfiguration;
import alluxio.security.authorization.AccessControlList;
import alluxio.security.authorization.AclEntry;
import alluxio.security.authorization.DefaultAccessControlList;
@@ -56,20 +55,15 @@
/** UFS Configuration options. */
protected final UnderFileSystemConfiguration mUfsConf;

protected final AlluxioConfiguration mAlluxioConf;

/**
* Constructs an {@link BaseUnderFileSystem}.
*
* @param uri the {@link AlluxioURI} used to create this ufs
* @param ufsConf UFS configuration
* @param alluxioConf Alluxio configuration
*/
protected BaseUnderFileSystem(AlluxioURI uri, UnderFileSystemConfiguration ufsConf,
AlluxioConfiguration alluxioConf) {
protected BaseUnderFileSystem(AlluxioURI uri, UnderFileSystemConfiguration ufsConf) {
mUri = Preconditions.checkNotNull(uri, "uri");
mUfsConf = Preconditions.checkNotNull(ufsConf, "ufsConf");
mAlluxioConf = Preconditions.checkNotNull(alluxioConf, "alluxioConf");
}

@Override
@@ -12,7 +12,6 @@
package alluxio.underfs;

import alluxio.AlluxioURI;
import alluxio.conf.AlluxioConfiguration;
import alluxio.underfs.options.CreateOptions;
import alluxio.underfs.options.DeleteOptions;
import alluxio.underfs.options.OpenOptions;
@@ -32,11 +31,9 @@
*
* @param uri path belonging to this under file system
* @param ufsConf UFS configuration
* @param alluxioConf Alluxio configuration
*/
public ConsistentUnderFileSystem(AlluxioURI uri,
UnderFileSystemConfiguration ufsConf, AlluxioConfiguration alluxioConf) {
super(uri, ufsConf, alluxioConf);
public ConsistentUnderFileSystem(AlluxioURI uri, UnderFileSystemConfiguration ufsConf) {
super(uri, ufsConf);
}

@Override
@@ -81,10 +81,9 @@
* @param uri the {@link AlluxioURI} used to create this ufs
* @param ufsConf UFS configuration
*/
protected ObjectUnderFileSystem(AlluxioURI uri, UnderFileSystemConfiguration ufsConf,
AlluxioConfiguration alluxioConf) {
super(uri, ufsConf, alluxioConf);
int numThreads = mAlluxioConf.getInt(PropertyKey.UNDERFS_OBJECT_STORE_SERVICE_THREADS);
protected ObjectUnderFileSystem(AlluxioURI uri, UnderFileSystemConfiguration ufsConf) {
super(uri, ufsConf);
int numThreads = mUfsConf.getInt(PropertyKey.UNDERFS_OBJECT_STORE_SERVICE_THREADS);
mExecutorService = ExecutorServiceFactories.fixedThreadPool(
"alluxio-underfs-object-service-worker", numThreads).create();
}
@@ -459,7 +458,7 @@ public DeleteBuffer() {}
@Override
protected int getBatchSize() {
// Delete batch size is same as listing length
return getListingChunkLength(mAlluxioConf);
return getListingChunkLength(mUfsConf);
}

@Override
@@ -476,7 +475,7 @@ protected int getBatchSize() {
*/
@Override
public long getBlockSizeByte(String path) throws IOException {
return mAlluxioConf.getBytes(PropertyKey.USER_BLOCK_SIZE_BYTES_DEFAULT);
return mUfsConf.getBytes(PropertyKey.USER_BLOCK_SIZE_BYTES_DEFAULT);
}

@Override
@@ -34,7 +34,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -65,26 +64,16 @@

private Factory() {} // prevent instantiation

/**
* Creates the {@link UnderFileSystem} instance according to its UFS path. This method should
* only be used for journal operations and tests.
*
* @param path the file path storing over the ufs
* @return instance of the under layer file system
*/
public static UnderFileSystem create(String path, AlluxioConfiguration conf) {
return create(path, UnderFileSystemConfiguration.defaults(conf), conf);
}

/**
* Creates the {@link UnderFileSystem} instance according to its UFS path. This method should
* only be used for journal operations and tests.
*
* @param path journal path in ufs
* @param conf the configuration object w/o mount specific options
* @return the instance of under file system for Alluxio journal directory
*/
public static UnderFileSystem create(URI path, AlluxioConfiguration conf) {
return create(path.toString(), conf);
public static UnderFileSystem create(String path, AlluxioConfiguration conf) {
return create(path, UnderFileSystemConfiguration.defaults(conf));
}

/**
@@ -93,15 +82,13 @@ public static UnderFileSystem create(URI path, AlluxioConfiguration conf) {
* path or if no under file system could successfully be created.
*
* @param path path
* @param ufsConf optional configuration object for the UFS, may be null
* @param alluxioConf Alluxio configuration
* @param ufsConf configuration object for the UFS
* @return client for the under file system
*/
public static UnderFileSystem create(String path, UnderFileSystemConfiguration ufsConf,
AlluxioConfiguration alluxioConf) {
public static UnderFileSystem create(String path, UnderFileSystemConfiguration ufsConf) {
// Try to obtain the appropriate factory
List<UnderFileSystemFactory> factories =
UnderFileSystemFactoryRegistry.findAll(path, ufsConf, alluxioConf);
UnderFileSystemFactoryRegistry.findAll(path, ufsConf);
if (factories.isEmpty()) {
throw new IllegalArgumentException("No Under File System Factory found for: " + path);
}
@@ -115,8 +102,7 @@ public static UnderFileSystem create(String path, UnderFileSystemConfiguration u
// when creation is done.
Thread.currentThread().setContextClassLoader(factory.getClass().getClassLoader());
// Use the factory to create the actual client for the Under File System
return new UnderFileSystemWithLogging(path, factory.create(path, ufsConf, alluxioConf),
alluxioConf);
return new UnderFileSystemWithLogging(path, factory.create(path, ufsConf), ufsConf);
} catch (Throwable e) {
// Catching Throwable rather than Exception to catch service loading errors
errors.add(e);
@@ -17,7 +17,6 @@
import alluxio.conf.ConfigurationValueOptions;
import alluxio.conf.InstancedConfiguration;
import alluxio.conf.Source;
import alluxio.util.ConfigurationUtils;

import java.util.HashMap;
import java.util.Map;
@@ -45,13 +44,6 @@
private boolean mReadOnly;
private boolean mShared;

/**
* @return default UFS configuration
*/
public static UnderFileSystemConfiguration defaults() {
return new UnderFileSystemConfiguration(ConfigurationUtils.defaults());
}

/**
* @param alluxioConf Alluxio configuration
* @return ufs configuration from a given alluxio configuration
@@ -12,11 +12,8 @@
package alluxio.underfs;

import alluxio.annotation.PublicApi;
import alluxio.conf.AlluxioConfiguration;
import alluxio.extensions.ExtensionFactory;

import javax.annotation.Nullable;

/**
* Interface for under file system factories.
*/
@@ -30,16 +27,14 @@
* provided is insufficient to create a client.
*
* @param path file path
* @param conf optional configuration object for the UFS, may be null
* @param alluxioConf configuration object for alluxio
* @param conf configuration object for the UFS
* @return the client
*/
UnderFileSystem create(String path, @Nullable UnderFileSystemConfiguration conf,
AlluxioConfiguration alluxioConf);
UnderFileSystem create(String path, UnderFileSystemConfiguration conf);

/**
* Gets whether this factory supports the given path and thus whether calling the
* {@link #create(String, UnderFileSystemConfiguration, AlluxioConfiguration)} can succeed for
* {@link #create(String, UnderFileSystemConfiguration)} can succeed for
* this path.
*
* @param path file path
@@ -49,14 +44,14 @@ UnderFileSystem create(String path, @Nullable UnderFileSystemConfiguration conf,

/**
* Gets whether this factory supports the given path and thus whether calling the
* {@link #create(String, UnderFileSystemConfiguration, AlluxioConfiguration)} can succeed for
* {@link #create(String, UnderFileSystemConfiguration)} can succeed for
* this path.
*
* @param path file path
* @param conf optional configuration object for the UFS, may be null
* @param conf configuration object for the UFS
* @return true if the path is supported, false otherwise
*/
default boolean supportsPath(String path, @Nullable UnderFileSystemConfiguration conf) {
default boolean supportsPath(String path, UnderFileSystemConfiguration conf) {
return supportsPath(path);
}

@@ -73,22 +73,20 @@ private UnderFileSystemFactoryRegistry() {}
*/
@Nullable
public static UnderFileSystemFactory find(String path, AlluxioConfiguration alluxioConf) {
return find(path, null, alluxioConf);
return find(path, UnderFileSystemConfiguration.defaults(alluxioConf));
}

/**
* Finds the first Under File System factory that supports the given path.
*
* @param path path
* @param ufsConf optional configuration object for the UFS, may be null
* @param alluxioConf Alluxio configuration
* @param ufsConf configuration object for the UFS
* @return factory if available, null otherwise
*/
@Nullable
public static UnderFileSystemFactory find(
String path, @Nullable UnderFileSystemConfiguration ufsConf,
AlluxioConfiguration alluxioConf) {
List<UnderFileSystemFactory> factories = findAll(path, ufsConf, alluxioConf);
String path, UnderFileSystemConfiguration ufsConf) {
List<UnderFileSystemFactory> 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);
@@ -104,28 +102,29 @@ public static UnderFileSystemFactory find(
*
* @param path path
* @param ufsConf configuration of the UFS
* @param alluxioConf Alluxio configuration
* @return list of factories that support the given path which may be an empty list
*/
public static List<UnderFileSystemFactory> findAll(String path,
UnderFileSystemConfiguration ufsConf, AlluxioConfiguration alluxioConf) {
List<UnderFileSystemFactory> eligibleFactories = sRegistryInstance.findAll(path, ufsConf,
alluxioConf);
if (eligibleFactories.isEmpty() && ufsConf != null) {
UnderFileSystemConfiguration ufsConf) {
List<UnderFileSystemFactory> eligibleFactories = sRegistryInstance.findAll(path, ufsConf);
if (eligibleFactories.isEmpty() && ufsConf.isSet(PropertyKey.UNDERFS_VERSION)) {
String configuredVersion = ufsConf.get(PropertyKey.UNDERFS_VERSION);
// Versioned factories ignore version if not set
ufsConf.unset(PropertyKey.UNDERFS_VERSION);
// Check if any versioned factory supports the default configuration
List<UnderFileSystemFactory> factories = sRegistryInstance.findAll(path, null, alluxioConf);
List<UnderFileSystemFactory> factories = sRegistryInstance.findAll(path, ufsConf);
List<String> supportedVersions = new java.util.ArrayList<>();
for (UnderFileSystemFactory factory : factories) {
if (!factory.getVersion().isEmpty()) {
supportedVersions.add(factory.getVersion());
}
}
if (!supportedVersions.isEmpty()) {
String configuredVersion = ufsConf.get(PropertyKey.UNDERFS_VERSION);
LOG.warn("Versions [{}] are supported for path {} but you have configured version: {}",
StringUtils.join(supportedVersions, ","), path,
configuredVersion);
}
ufsConf.set(PropertyKey.UNDERFS_VERSION, configuredVersion);
}
return eligibleFactories;
}

0 comments on commit d95cdea

Please sign in to comment.
You can’t perform that action at this time.