HBASE-21062 Correctly use the defaultProvider value on the Providers enum when constructing a WALProvider

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

@@ -119,22 +119,32 @@ public class WALFactory {
     factoryId = SINGLETON_ID;
   }
 
+  @VisibleForTesting
+  Providers getDefaultProvider() {
+    return Providers.defaultProvider;
+  }
+
   @VisibleForTesting
   public Class<? extends WALProvider> getProviderClass(String key, String defaultValue) {
     try {
       Providers provider = Providers.valueOf(conf.get(key, defaultValue));
-      if (provider != Providers.defaultProvider) {
+
-        // User gives a wal provider explicitly, just use that one
+      // AsyncFSWALProvider is not guaranteed to work on all Hadoop versions, when it's chosen as
-        return provider.clazz;
+      // the default and we can't us it, we want to fall back to FSHLog which we know works on
-      }
+      // all versions.
-      // AsyncFSWAL has better performance in most cases, and also uses less resources, we will try
+      if (provider == getDefaultProvider() && provider.clazz == AsyncFSWALProvider.class
-      // to use it if possible. But it deeply hacks into the internal of DFSClient so will be easily
+          && !AsyncFSWALProvider.load()) {
-      // broken when upgrading hadoop. If it is broken, then we fall back to use FSHLog.
+        // AsyncFSWAL has better performance in most cases, and also uses less resources, we will
-      if (AsyncFSWALProvider.load()) {
+        // try to use it if possible. It deeply hacks into the internal of DFSClient so will be
-        return AsyncFSWALProvider.class;
+        // easily broken when upgrading hadoop.
-      } else {
+        LOG.warn("Failed to load AsyncFSWALProvider, falling back to FSHLogProvider");
         return FSHLogProvider.class;
       }
+
+      // N.b. If the user specifically requested AsyncFSWALProvider but their environment doesn't
+      // support using it (e.g. AsyncFSWALProvider.load() == false), we should let this fail and
+      // not fall back to FSHLogProvider.
+      return provider.clazz;
     } catch (IllegalArgumentException exception) {
       // Fall back to them specifying a class name
       // Note that the passed default class shouldn't actually be used, since the above only fails

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

@@ -62,6 +62,7 @@ import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.CommonFSUtils;
 import org.apache.hadoop.hbase.util.FSUtils;
 import org.apache.hadoop.hbase.util.Threads;
+import org.apache.hadoop.hbase.wal.WALFactory.Providers;
 import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
@@ -719,4 +720,26 @@ public class TestWALFactory {
     assertEquals(WALFactory.Providers.asyncfs.clazz, walFactory.getMetaProvider().getClass());
   }
 
+  @Test
+  public void testDefaultProvider() throws IOException {
+    final Configuration conf = new Configuration();
+    // 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, Providers.filesystem.name());
+    assertEquals(Providers.filesystem.clazz, fshLogProvider);
+
+    // Imagine a world where MultiWAL is the default
+    final WALFactory customizedWalFactory = new WALFactory(
+        conf, this.currentServername.toString())  {
+      @Override
+      Providers getDefaultProvider() {
+        return Providers.multiwal;
+      }
+    };
+    // If we don't specify a WALProvider, we should get the default implementation.
+    Class<? extends WALProvider> multiwalProviderClass = customizedWalFactory.getProviderClass(
+        WALFactory.WAL_PROVIDER, Providers.multiwal.name());
+    assertEquals(Providers.multiwal.clazz, multiwalProviderClass);
+  }
 }