From 1d0fca370bf56a41fc62b72bebe86a7185a2b0c2 Mon Sep 17 00:00:00 2001 From: TAK LON WU Date: Mon, 16 Jul 2018 15:22:16 -0700 Subject: [PATCH] HBASE-20856 PITA having to set WAL provider in two places With this change if hbase.wal.meta_provider is not explicitly set, it uses whatever set with hbase.wal.provider. this change avoids a use case of unexpectedly using two different providers when only hbase.wal.provider is set to non-default but not hbase.wal.meta_provider. This change also include document (architecture.adoc) update Signed-off-by: Zach York Signed-off-by: Michael Stack Signed-off-by: Sean Busbey Signed-off-by: Duo Zhang --- .../hbase/wal/SyncReplicationWALProvider.java | 6 +++ .../apache/hadoop/hbase/wal/WALFactory.java | 7 +-- .../TestShutdownWhileWALBroken.java | 1 - .../regionserver/wal/TestAsyncLogRolling.java | 1 - .../regionserver/wal/TestLogRolling.java | 1 - .../hadoop/hbase/wal/TestWALFactory.java | 46 +++++++++++++++++++ src/main/asciidoc/_chapters/architecture.adoc | 3 +- 7 files changed, 58 insertions(+), 7 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/SyncReplicationWALProvider.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/SyncReplicationWALProvider.java index b9fffcff601..9859c204649 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/SyncReplicationWALProvider.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/SyncReplicationWALProvider.java @@ -343,4 +343,10 @@ public class SyncReplicationWALProvider implements WALProvider, PeerActionListen return Optional.empty(); } } + + @VisibleForTesting + WALProvider getWrappedProvider() { + return provider; + } + } 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 8eb169022a5..24ebe6808ab 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 @@ -82,7 +82,6 @@ public class WALFactory { static final String DEFAULT_WAL_PROVIDER = Providers.defaultProvider.name(); public static final String META_WAL_PROVIDER = "hbase.wal.meta_provider"; - static final String DEFAULT_META_WAL_PROVIDER = Providers.defaultProvider.name(); final String factoryId; private final WALProvider provider; @@ -244,13 +243,15 @@ public class WALFactory { return provider.getWALs(); } - private WALProvider getMetaProvider() throws IOException { + @VisibleForTesting + WALProvider getMetaProvider() throws IOException { for (;;) { WALProvider provider = this.metaProvider.get(); if (provider != null) { return provider; } - provider = createProvider(getProviderClass(META_WAL_PROVIDER, DEFAULT_META_WAL_PROVIDER)); + provider = createProvider(getProviderClass(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)) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java index 6c9b5e358c8..f1b090d4a6c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java @@ -116,7 +116,6 @@ public class TestShutdownWhileWALBroken { UTIL.getConfiguration().setClass(HConstants.REGION_SERVER_IMPL, MyRegionServer.class, HRegionServer.class); UTIL.getConfiguration().set(WALFactory.WAL_PROVIDER, walType); - UTIL.getConfiguration().set(WALFactory.META_WAL_PROVIDER, walType); UTIL.startMiniCluster(2); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncLogRolling.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncLogRolling.java index 992e6a52410..8afae061be4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncLogRolling.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestAsyncLogRolling.java @@ -48,7 +48,6 @@ public class TestAsyncLogRolling extends AbstractTestLogRolling { Configuration conf = TestAsyncLogRolling.TEST_UTIL.getConfiguration(); conf.setInt(FanOutOneBlockAsyncDFSOutputHelper.ASYNC_DFS_OUTPUT_CREATE_MAX_RETRIES, 100); conf.set(WALFactory.WAL_PROVIDER, "asyncfs"); - conf.set(WALFactory.META_WAL_PROVIDER, "asyncfs"); AbstractTestLogRolling.setUpBeforeClass(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java index f3cf2bfe643..e19361e2003 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java @@ -90,7 +90,6 @@ public class TestLogRolling extends AbstractTestLogRolling { conf.setInt("hbase.regionserver.hlog.tolerable.lowreplication", 2); conf.setInt("hbase.regionserver.hlog.lowreplication.rolllimit", 3); conf.set(WALFactory.WAL_PROVIDER, "filesystem"); - conf.set(WALFactory.META_WAL_PROVIDER, "filesystem"); AbstractTestLogRolling.setUpBeforeClass(); } 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 fd2b3c49856..216407ab44b 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 @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hbase.wal; +import static org.apache.hadoop.hbase.wal.WALFactory.META_WAL_PROVIDER; +import static org.apache.hadoop.hbase.wal.WALFactory.WAL_PROVIDER; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -673,4 +675,48 @@ public class TestWALFactory { increments++; } } + + @Test + public void testWALProviders() throws IOException { + Configuration conf = new Configuration(); + // 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()); + assertEquals(SyncReplicationWALProvider.class, walFactory.getWALProvider().getClass()); + WALProvider wrappedWALProvider = ((SyncReplicationWALProvider) walFactory.getWALProvider()) + .getWrappedProvider(); + assertEquals(wrappedWALProvider.getClass(), walFactory.getMetaProvider().getClass()); + + // if providers are not set and do not enable SyncReplicationWALProvider + walFactory = new WALFactory(conf, this.currentServername.toString(), false); + assertEquals(walFactory.getWALProvider().getClass(), walFactory.getMetaProvider().getClass()); + } + + @Test + public void testOnlySetWALProvider() throws IOException { + Configuration conf = new Configuration(); + conf.set(WAL_PROVIDER, WALFactory.Providers.multiwal.name()); + 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(WALFactory.Providers.multiwal.clazz, wrappedWALProvider.getClass()); + assertEquals(WALFactory.Providers.multiwal.clazz, walFactory.getMetaProvider().getClass()); + } + + @Test + public void testOnlySetMetaWALProvider() throws IOException { + Configuration conf = new Configuration(); + conf.set(META_WAL_PROVIDER, WALFactory.Providers.asyncfs.name()); + WALFactory walFactory = new WALFactory(conf, this.currentServername.toString()); + WALProvider wrappedWALProvider = ((SyncReplicationWALProvider) walFactory.getWALProvider()) + .getWrappedProvider(); + + assertEquals(SyncReplicationWALProvider.class, walFactory.getWALProvider().getClass()); + assertEquals(WALFactory.Providers.defaultProvider.clazz, wrappedWALProvider.getClass()); + assertEquals(WALFactory.Providers.asyncfs.clazz, walFactory.getMetaProvider().getClass()); + } + } diff --git a/src/main/asciidoc/_chapters/architecture.adoc b/src/main/asciidoc/_chapters/architecture.adoc index c2d158c741f..43ac100d6ef 100644 --- a/src/main/asciidoc/_chapters/architecture.adoc +++ b/src/main/asciidoc/_chapters/architecture.adoc @@ -1043,7 +1043,8 @@ In HBase, there are a number of WAL imlementations (or 'Providers'). Each is kno by a short name label (that unfortunately is not always descriptive). You set the provider in _hbase-site.xml_ passing the WAL provder short-name as the value on the _hbase.wal.provider_ property (Set the provider for _hbase:meta_ using the -_hbase.wal.meta_provider_ property). +_hbase.wal.meta_provider_ property, otherwise it uses the same provider configured +by _hbase.wal.provider_). * _asyncfs_: The *default*. New since hbase-2.0.0 (HBASE-15536, HBASE-14790). This _AsyncFSWAL_ provider, as it identifies itself in RegionServer logs, is built on a new non-blocking dfsclient implementation. It is currently resident in the hbase codebase but intent is to move it back up into HDFS itself. WALs edits are written concurrently ("fan-out") style to each of the WAL-block replicas on each DataNode rather than in a chained pipeline as the default client does. Latencies should be better. See link:https://www.slideshare.net/HBaseCon/apache-hbase-improvements-and-practices-at-xiaomi[Apache HBase Improements and Practices at Xiaomi] at slide 14 onward for more detail on implementation. * _filesystem_: This was the default in hbase-1.x releases. It is built on the blocking _DFSClient_ and writes to replicas in classic _DFSCLient_ pipeline mode. In logs it identifies as _FSHLog_ or _FSHLogProvider_.