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 <zyork@apache.org>
Signed-off-by: Michael Stack <stack@apache.org>
Signed-off-by: Sean Busbey <busbey@apache.org>
Signed-off-by: Duo Zhang <Apache9@apache.org>

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;
+  }
+
 }

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)) {

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);
   }
 

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();
   }
 

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();
   }
 

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());
+  }
+
 }

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_.