From 1ae3d2a1386a4b46ca47ef2c584077d37de0eeab Mon Sep 17 00:00:00 2001 From: Josh Elser Date: Wed, 15 Aug 2018 15:25:56 -0400 Subject: [PATCH] HBASE-21062 Correctly use the defaultProvider value on the Providers enum when constructing a WALProvider --- .../apache/hadoop/hbase/wal/WALFactory.java | 20 +++++++++---------- .../hadoop/hbase/wal/TestWALFactory.java | 16 +++++++++++++++ 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALFactory.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALFactory.java index 0118eab8e2..acb2e4df79 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALFactory.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALFactory.java @@ -119,22 +119,22 @@ public class WALFactory { factoryId = SINGLETON_ID; } + @VisibleForTesting + Providers getDefaultProvider() { + return Providers.defaultProvider; + } + @VisibleForTesting public Class 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 - return provider.clazz; - } - // AsyncFSWAL has better performance in most cases, and also uses less resources, we will try - // to use it if possible. But it deeply hacks into the internal of DFSClient so will be easily - // broken when upgrading hadoop. If it is broken, then we fall back to use FSHLog. - if (AsyncFSWALProvider.load()) { - return AsyncFSWALProvider.class; - } else { + if (provider.clazz == AsyncFSWALProvider.class && !AsyncFSWALProvider.load()) { + // AsyncFSWAL has better performance in most cases, and also uses less resources, we will try + // to use it if possible. But it deeply hacks into the internal of DFSClient so will be easily + // broken when upgrading hadoop. If it is broken, then we fall back to use FSHLog. return FSHLogProvider.class; } + 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 diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALFactory.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALFactory.java index fea2b1feb5..55b9b6b419 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALFactory.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALFactory.java @@ -61,6 +61,7 @@ import org.apache.hadoop.hbase.testclassification.RegionServerTests; import org.apache.hadoop.hbase.util.Bytes; 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; @@ -702,4 +703,19 @@ public class TestWALFactory { assertEquals(WALFactory.Providers.asyncfs.clazz, walFactory.getMetaProvider().getClass()); } + @Test + public void testDefaultProvider() throws IOException { + final Configuration conf = new Configuration(); + // Imagine a world where MultiWAL is the default + final WALFactory walFactory = 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 providerClass = walFactory.getProviderClass( + WALFactory.WAL_PROVIDER, Providers.multiwal.name()); + assertEquals(Providers.multiwal.clazz, providerClass); + } } -- 2.18.0