HBASE-21062 Correctly use the defaultProvider value on the Providers enum when constructing a WALProvider
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/4d7ed0f9 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/4d7ed0f9 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/4d7ed0f9 Branch: refs/heads/master Commit: 4d7ed0f94c8dc02522e2629c5bc6cd85421c4bce Parents: 092efb4 Author: Josh Elser <[email protected]> Authored: Wed Aug 15 15:25:56 2018 -0400 Committer: Josh Elser <[email protected]> Committed: Thu Aug 16 10:23:03 2018 -0400 ---------------------------------------------------------------------- .../org/apache/hadoop/hbase/wal/WALFactory.java | 30 +++++++++++++------- .../apache/hadoop/hbase/wal/TestWALFactory.java | 23 +++++++++++++++ 2 files changed, 43 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/4d7ed0f9/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALFactory.java ---------------------------------------------------------------------- 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 24ebe68..7b2cdbb 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 @@ -120,21 +120,31 @@ public class WALFactory { } @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 - 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 { + + // AsyncFSWALProvider is not guaranteed to work on all Hadoop versions, when it's chosen as + // the default and we can't us it, we want to fall back to FSHLog which we know works on + // all versions. + if (provider == getDefaultProvider() && 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. It deeply hacks into the internal of DFSClient so will be + // easily broken when upgrading hadoop. + 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 http://git-wip-us.apache.org/repos/asf/hbase/blob/4d7ed0f9/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALFactory.java ---------------------------------------------------------------------- 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 216407a..d19265f 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 @@ -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); + } }
