HBASE-21247 Allow WAL Provider to be specified by configuration without explicit enum in Providers - revert

hbase-server/src/main/java/org/apache/hadoop/hbase/wal/RegionGroupingProvider.java

@@ -121,8 +121,6 @@ public class RegionGroupingProvider implements WALProvider {
 
   /** delegate provider for WAL creation/roll/close */
   public static final String DELEGATE_PROVIDER = "hbase.wal.regiongrouping.delegate.provider";
-  public static final String DELEGATE_PROVIDER_CLASS =
-            "hbase.wal.regiongrouping.delegate.provider.class";
   public static final String DEFAULT_DELEGATE_PROVIDER = WALFactory.Providers.defaultProvider
       .name();
 
@@ -157,8 +155,7 @@ public class RegionGroupingProvider implements WALProvider {
     }
     this.providerId = sb.toString();
     this.strategy = getStrategy(conf, REGION_GROUPING_STRATEGY, DEFAULT_REGION_GROUPING_STRATEGY);
-    this.providerClass = factory.getProviderClass(DELEGATE_PROVIDER_CLASS, DELEGATE_PROVIDER,
+    this.providerClass = factory.getProviderClass(DELEGATE_PROVIDER, DEFAULT_DELEGATE_PROVIDER);
-        DEFAULT_DELEGATE_PROVIDER);
   }
 
   private WALProvider createProvider(String group) throws IOException {

hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALFactory.java

@@ -80,11 +80,8 @@ public class WALFactory {
 
   public static final String WAL_PROVIDER = "hbase.wal.provider";
   static final String DEFAULT_WAL_PROVIDER = Providers.defaultProvider.name();
-  public static final String WAL_PROVIDER_CLASS = "hbase.wal.provider.class";
-  static final Class<? extends WALProvider> DEFAULT_WAL_PROVIDER_CLASS = AsyncFSWALProvider.class;
 
   public static final String META_WAL_PROVIDER = "hbase.wal.meta_provider";
-  public static final String META_WAL_PROVIDER_CLASS = "hbase.wal.meta_provider.class";
 
   final String factoryId;
   private final WALProvider provider;
@@ -128,25 +125,7 @@ public class WALFactory {
   }
 
   @VisibleForTesting
-  /*
+  public Class<? extends WALProvider> getProviderClass(String key, String defaultValue) {
-   * @param clsKey config key for provider classname
-   * @param key config key for provider enum
-   * @param defaultValue default value for provider enum
-   * @return Class which extends WALProvider
-   */
-  public Class<? extends WALProvider> getProviderClass(String clsKey, String key,
-      String defaultValue) {
-    String clsName = conf.get(clsKey);
-    if (clsName == null || clsName.isEmpty()) {
-      clsName = conf.get(key, defaultValue);
-    }
-    if (clsName != null && !clsName.isEmpty()) {
-      try {
-        return (Class<? extends WALProvider>) Class.forName(clsName);
-      } catch (ClassNotFoundException exception) {
-        // try with enum key next
-      }
-    }
     try {
       Providers provider = Providers.valueOf(conf.get(key, defaultValue));
 
@@ -170,7 +149,7 @@ public class WALFactory {
       // Fall back to them specifying a class name
       // Note that the passed default class shouldn't actually be used, since the above only fails
       // when there is a config value present.
-      return conf.getClass(key, AsyncFSWALProvider.class, WALProvider.class);
+      return conf.getClass(key, Providers.defaultProvider.clazz, WALProvider.class);
     }
   }
 
@@ -217,8 +196,7 @@ public class WALFactory {
     this.factoryId = factoryId;
     // end required early initialization
     if (conf.getBoolean("hbase.regionserver.hlog.enabled", true)) {
-      WALProvider provider = createProvider(
+      WALProvider provider = createProvider(getProviderClass(WAL_PROVIDER, DEFAULT_WAL_PROVIDER));
-          getProviderClass(WAL_PROVIDER_CLASS, WAL_PROVIDER, DEFAULT_WAL_PROVIDER));
       if (enableSyncReplicationWALProvider) {
         provider = new SyncReplicationWALProvider(provider);
       }
@@ -282,10 +260,8 @@ public class WALFactory {
       if (provider != null) {
         return provider;
       }
-      boolean metaWALProvPresent = conf.get(META_WAL_PROVIDER_CLASS) != null;
+      provider = createProvider(getProviderClass(META_WAL_PROVIDER,
-      provider = createProvider(getProviderClass(
+          conf.get(WAL_PROVIDER, DEFAULT_WAL_PROVIDER)));
-          metaWALProvPresent ? META_WAL_PROVIDER_CLASS : WAL_PROVIDER_CLASS,
-          META_WAL_PROVIDER, conf.get(WAL_PROVIDER, DEFAULT_WAL_PROVIDER)));
       provider.init(this, conf, AbstractFSWALProvider.META_WAL_PROVIDER_ID);
       provider.addWALActionsListener(new MetricsWAL());
       if (metaProvider.compareAndSet(null, provider)) {

hbase-server/src/test/java/org/apache/hadoop/hbase/wal/IOTestProvider.java

@@ -27,8 +27,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicBoolean;
-
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
@@ -89,7 +87,6 @@ public class IOTestProvider implements WALProvider {
   private volatile FSHLog log;
 
   private String providerId;
-  protected AtomicBoolean initialized = new AtomicBoolean(false);
 
   private List<WALActionsListener> listeners = new ArrayList<>();
   /**
@@ -100,7 +97,7 @@ public class IOTestProvider implements WALProvider {
    */
   @Override
   public void init(WALFactory factory, Configuration conf, String providerId) throws IOException {
-    if (!initialized.compareAndSet(false, true)) {
+    if (factory != null) {
       throw new IllegalStateException("WALProvider.init should only be called once.");
     }
     this.factory = factory;

hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALFactory.java

@@ -680,7 +680,6 @@ public class TestWALFactory {
   @Test
   public void testWALProviders() throws IOException {
     Configuration conf = new Configuration();
-    conf.set(HConstants.HBASE_DIR, TestWALFactory.conf.get(HConstants.HBASE_DIR));
     // if providers are not set but enable SyncReplicationWALProvider by default for master node
     // with not only system tables
     WALFactory walFactory = new WALFactory(conf, this.currentServername.toString());
@@ -697,30 +696,28 @@ public class TestWALFactory {
   @Test
   public void testOnlySetWALProvider() throws IOException {
     Configuration conf = new Configuration();
-    conf.set(WAL_PROVIDER, RegionGroupingProvider.class.getName());
+    conf.set(WAL_PROVIDER, WALFactory.Providers.multiwal.name());
-    conf.set(HConstants.HBASE_DIR, TestWALFactory.conf.get(HConstants.HBASE_DIR));
     WALFactory walFactory = new WALFactory(conf, this.currentServername.toString());
     WALProvider wrappedWALProvider = ((SyncReplicationWALProvider) walFactory.getWALProvider())
         .getWrappedProvider();
 
     assertEquals(SyncReplicationWALProvider.class, walFactory.getWALProvider().getClass());
     // class of WALProvider and metaWALProvider are the same when metaWALProvider is not set
-    assertEquals(RegionGroupingProvider.class, wrappedWALProvider.getClass());
+    assertEquals(WALFactory.Providers.multiwal.clazz, wrappedWALProvider.getClass());
-    assertEquals(RegionGroupingProvider.class, walFactory.getMetaProvider().getClass());
+    assertEquals(WALFactory.Providers.multiwal.clazz, walFactory.getMetaProvider().getClass());
   }
 
   @Test
   public void testOnlySetMetaWALProvider() throws IOException {
     Configuration conf = new Configuration();
-    conf.set(META_WAL_PROVIDER, AsyncFSWALProvider.class.getName());
+    conf.set(META_WAL_PROVIDER, WALFactory.Providers.asyncfs.name());
-    conf.set(HConstants.HBASE_DIR, TestWALFactory.conf.get(HConstants.HBASE_DIR));
     WALFactory walFactory = new WALFactory(conf, this.currentServername.toString());
     WALProvider wrappedWALProvider = ((SyncReplicationWALProvider) walFactory.getWALProvider())
         .getWrappedProvider();
 
     assertEquals(SyncReplicationWALProvider.class, walFactory.getWALProvider().getClass());
-    assertEquals(AsyncFSWALProvider.class, wrappedWALProvider.getClass());
+    assertEquals(WALFactory.Providers.defaultProvider.clazz, wrappedWALProvider.getClass());
-    assertEquals(AsyncFSWALProvider.class, walFactory.getMetaProvider().getClass());
+    assertEquals(WALFactory.Providers.asyncfs.clazz, walFactory.getMetaProvider().getClass());
   }
 
   @Test
@@ -729,7 +726,7 @@ public class TestWALFactory {
     // AsyncFSWal is the default, we should be able to request any WAL.
     final WALFactory normalWalFactory = new WALFactory(conf, this.currentServername.toString());
     Class<? extends WALProvider> fshLogProvider = normalWalFactory.getProviderClass(
-        WALFactory.WAL_PROVIDER_CLASS, WALFactory.WAL_PROVIDER, Providers.filesystem.name());
+        WALFactory.WAL_PROVIDER, Providers.filesystem.name());
     assertEquals(Providers.filesystem.clazz, fshLogProvider);
 
     // Imagine a world where MultiWAL is the default
@@ -742,31 +739,7 @@ public class TestWALFactory {
     };
     // If we don't specify a WALProvider, we should get the default implementation.
     Class<? extends WALProvider> multiwalProviderClass = customizedWalFactory.getProviderClass(
-        WALFactory.WAL_PROVIDER_CLASS, WALFactory.WAL_PROVIDER, Providers.multiwal.name());
+        WALFactory.WAL_PROVIDER, Providers.multiwal.name());
     assertEquals(Providers.multiwal.clazz, multiwalProviderClass);
   }
-
-  @Test
-  public void testCustomProvider() throws IOException {
-    final Configuration config = new Configuration();
-    config.set(WALFactory.WAL_PROVIDER_CLASS, IOTestProvider.class.getName());
-    final WALFactory walFactory = new WALFactory(config, this.currentServername.toString());
-    Class<? extends WALProvider> walProvider = walFactory.getProviderClass(
-        WALFactory.WAL_PROVIDER_CLASS, WALFactory.WAL_PROVIDER, Providers.filesystem.name());
-    assertEquals(IOTestProvider.class, walProvider);
-    WALProvider metaWALProvider = walFactory.getMetaProvider();
-    assertEquals(IOTestProvider.class, metaWALProvider.getClass());
-  }
-
-  @Test
-  public void testCustomMetaProvider() throws IOException {
-    final Configuration config = new Configuration();
-    config.set(WALFactory.META_WAL_PROVIDER_CLASS, IOTestProvider.class.getName());
-    final WALFactory walFactory = new WALFactory(config, this.currentServername.toString());
-    Class<? extends WALProvider> walProvider = walFactory.getProviderClass(
-        WALFactory.WAL_PROVIDER_CLASS, WALFactory.WAL_PROVIDER, Providers.filesystem.name());
-    assertEquals(Providers.filesystem.clazz, walProvider);
-    WALProvider metaWALProvider = walFactory.getMetaProvider();
-    assertEquals(IOTestProvider.class, metaWALProvider.getClass());
-  }
 }