HBASE-27702 Remove 'hbase.regionserver.hlog.writer.impl' config

hbase-it/src/test/java/org/apache/hadoop/hbase/IntegrationTestIngestWithEncryption.java

@@ -26,10 +26,8 @@
 import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.io.crypto.KeyProviderForTesting;
 import org.apache.hadoop.hbase.io.hfile.HFile;
-import org.apache.hadoop.hbase.regionserver.wal.SecureProtobufLogWriter;
 import org.apache.hadoop.hbase.testclassification.IntegrationTests;
 import org.apache.hadoop.hbase.util.EncryptionTest;
-import org.apache.hadoop.hbase.wal.WALProvider.Writer;
 import org.apache.hadoop.util.ToolRunner;
 import org.junit.Before;
 import org.junit.experimental.categories.Category;
@@ -52,8 +50,6 @@ public void setUpCluster() throws Exception {
       conf.setInt(HFile.FORMAT_VERSION_KEY, 3);
       conf.set(HConstants.CRYPTO_KEYPROVIDER_CONF_KEY, KeyProviderForTesting.class.getName());
       conf.set(HConstants.CRYPTO_MASTERKEY_NAME_CONF_KEY, "hbase");
-      conf.setClass("hbase.regionserver.hlog.writer.impl", SecureProtobufLogWriter.class,
-        Writer.class);
       conf.setBoolean(HConstants.ENABLE_WAL_ENCRYPTION, true);
     }
     // Check if the cluster configuration can support this test

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

@@ -29,7 +29,6 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.codec.Codec;
 import org.apache.hadoop.hbase.io.asyncfs.monitor.StreamSlowMonitor;
@@ -55,13 +54,14 @@
 /**
  * Base class for Protobuf log writer.
  */
-@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
+@InterfaceAudience.Private
 public abstract class AbstractProtobufLogWriter {
 
   private static final Logger LOG = LoggerFactory.getLogger(AbstractProtobufLogWriter.class);
 
   protected CompressionContext compressionContext;
   protected Configuration conf;
+  protected Encryptor encryptor;
   protected Codec.Encoder cellEncoder;
   protected WALCellCodec.ByteStringCompressor compressor;
   protected boolean trailerWritten;
@@ -77,64 +77,53 @@ private WALCellCodec getCodec(Configuration conf, CompressionContext compression
     return WALCellCodec.create(conf, null, compressionContext);
   }
 
-  private WALHeader buildWALHeader0(Configuration conf, WALHeader.Builder builder) {
-    if (!builder.hasWriterClsName()) {
-      builder.setWriterClsName(getWriterClassName());
-    }
-    if (!builder.hasCellCodecClsName()) {
-      builder.setCellCodecClsName(WALCellCodec.getWALCellCodecClass(conf).getName());
-    }
-    return builder.build();
-  }
-
-  protected WALHeader buildWALHeader(Configuration conf, WALHeader.Builder builder)
+  private WALHeader buildWALHeader(Configuration conf, WALHeader.Builder builder)
     throws IOException {
-    return buildWALHeader0(conf, builder);
+    builder.setWriterClsName(getWriterClassName());
+    builder.setCellCodecClsName(WALCellCodec.getWALCellCodecClass(conf).getName());
+    return builder.build();
   }
 
-  // should be called in sub classes's buildWALHeader method to build WALHeader for secure
-  // environment. Do not forget to override the setEncryptor method as it will be called in this
-  // method to init your encryptor.
-  protected final WALHeader buildSecureWALHeader(Configuration conf, WALHeader.Builder builder)
+  private WALHeader buildSecureWALHeader(Configuration conf, WALHeader.Builder builder)
     throws IOException {
-    builder.setWriterClsName(getWriterClassName());
-    if (conf.getBoolean(HConstants.ENABLE_WAL_ENCRYPTION, false)) {
-      EncryptionTest.testKeyProvider(conf);
-      EncryptionTest.testCipherProvider(conf);
-
-      // Get an instance of our cipher
-      final String cipherName =
-        conf.get(HConstants.CRYPTO_WAL_ALGORITHM_CONF_KEY, HConstants.CIPHER_AES);
-      Cipher cipher = Encryption.getCipher(conf, cipherName);
-      if (cipher == null) {
-        throw new RuntimeException("Cipher '" + cipherName + "' is not available");
-      }
+    EncryptionTest.testKeyProvider(conf);
+    EncryptionTest.testCipherProvider(conf);
+
+    // Get an instance of our cipher
+    final String cipherName =
+      conf.get(HConstants.CRYPTO_WAL_ALGORITHM_CONF_KEY, HConstants.CIPHER_AES);
+    Cipher cipher = Encryption.getCipher(conf, cipherName);
+    if (cipher == null) {
+      throw new RuntimeException("Cipher '" + cipherName + "' is not available");
+    }
 
-      // Generate a random encryption key for this WAL
-      Key key = cipher.getRandomKey();
-      builder.setEncryptionKey(UnsafeByteOperations.unsafeWrap(EncryptionUtil.wrapKey(conf,
-        conf.get(HConstants.CRYPTO_WAL_KEY_NAME_CONF_KEY,
-          conf.get(HConstants.CRYPTO_MASTERKEY_NAME_CONF_KEY, User.getCurrent().getShortName())),
-        key)));
-
-      // Set up the encryptor
-      Encryptor encryptor = cipher.getEncryptor();
-      encryptor.setKey(key);
-      setEncryptor(encryptor);
-      if (LOG.isTraceEnabled()) {
-        LOG.trace("Initialized secure protobuf WAL: cipher=" + cipher.getName());
-      }
+    // Generate a random encryption key for this WAL
+    Key key = cipher.getRandomKey();
+    builder.setEncryptionKey(UnsafeByteOperations.unsafeWrap(EncryptionUtil.wrapKey(conf,
+      conf.get(HConstants.CRYPTO_WAL_KEY_NAME_CONF_KEY,
+        conf.get(HConstants.CRYPTO_MASTERKEY_NAME_CONF_KEY, User.getCurrent().getShortName())),
+      key)));
+
+    // Set up the encryptor
+    Encryptor encryptor = cipher.getEncryptor();
+    encryptor.setKey(key);
+    this.encryptor = encryptor;
+    if (LOG.isTraceEnabled()) {
+      LOG.trace("Initialized secure protobuf WAL: cipher={}", cipher.getName());
     }
+    builder.setWriterClsName(getWriterClassName());
     builder.setCellCodecClsName(SecureWALCellCodec.class.getName());
-    return buildWALHeader0(conf, builder);
-  }
-
-  // override this if you need a encryptor
-  protected void setEncryptor(Encryptor encryptor) {
+    return builder.build();
   }
 
-  protected String getWriterClassName() {
-    return getClass().getSimpleName();
+  private String getWriterClassName() {
+    // class name which is recognized by hbase-1.x to avoid ProtobufLogReader throwing error:
+    // IOException: Got unknown writer class: AsyncProtobufLogWriter
+    if (encryptor == null) {
+      return "ProtobufLogWriter";
+    } else {
+      return "SecureProtobufLogWriter";
+    }
   }
 
   private boolean initializeCompressionContext(Configuration conf, Path path) throws IOException {
@@ -187,9 +176,13 @@ public void init(FileSystem fs, Path path, Configuration conf, boolean overwrita
         headerBuilder.setValueCompressionAlgorithm(
           CompressionContext.getValueCompressionAlgorithm(conf).ordinal());
       }
-      length.set(writeMagicAndWALHeader(PB_WAL_MAGIC, buildWALHeader(conf, headerBuilder)));
-
-      initAfterHeader(doCompress);
+      if (conf.getBoolean(HConstants.ENABLE_WAL_ENCRYPTION, false)) {
+        length.set(writeMagicAndWALHeader(PB_WAL_MAGIC, buildSecureWALHeader(conf, headerBuilder)));
+        secureInitAfterHeader(doCompress, encryptor);
+      } else {
+        length.set(writeMagicAndWALHeader(PB_WAL_MAGIC, buildWALHeader(conf, headerBuilder)));
+        initAfterHeader(doCompress);
+      }
 
       // instantiate trailer to default value.
       trailer = WALTrailer.newBuilder().build();
@@ -205,7 +198,7 @@ public void init(FileSystem fs, Path path, Configuration conf, boolean overwrita
     }
   }
 
-  private void initAfterHeader0(boolean doCompress) throws IOException {
+  private void initAfterHeader(boolean doCompress) throws IOException {
     WALCellCodec codec = getCodec(conf, this.compressionContext);
     this.cellEncoder = codec.getEncoder(getOutputStreamForCellEncoder());
     if (doCompress) {
@@ -215,21 +208,15 @@ private void initAfterHeader0(boolean doCompress) throws IOException {
     }
   }
 
-  protected void initAfterHeader(boolean doCompress) throws IOException {
-    initAfterHeader0(doCompress);
-  }
-
-  // should be called in sub classes's initAfterHeader method to init SecureWALCellCodec.
-  protected final void secureInitAfterHeader(boolean doCompress, Encryptor encryptor)
-    throws IOException {
-    if (conf.getBoolean(HConstants.ENABLE_WAL_ENCRYPTION, false) && encryptor != null) {
+  private void secureInitAfterHeader(boolean doCompress, Encryptor encryptor) throws IOException {
+    if (encryptor != null) {
       WALCellCodec codec = SecureWALCellCodec.getCodec(this.conf, encryptor);
       this.cellEncoder = codec.getEncoder(getOutputStreamForCellEncoder());
       // We do not support compression
       this.compressionContext = null;
       this.compressor = WALCellCodec.getNoneCompressor();
     } else {
-      initAfterHeader0(doCompress);
+      initAfterHeader(doCompress);
     }
   }
 
@@ -245,7 +232,7 @@ private WALTrailer buildWALTrailer(WALTrailer.Builder builder) {
     return builder.build();
   }
 
-  protected void writeWALTrailer() {
+  protected final void writeWALTrailer() {
     try {
       int trailerSize = 0;
       if (this.trailer == null) {

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

@@ -82,10 +82,9 @@ public abstract class AbstractProtobufWALReader
   static final String WAL_TRAILER_WARN_SIZE = "hbase.regionserver.waltrailer.warn.size";
   static final int DEFAULT_WAL_TRAILER_WARN_SIZE = 1024 * 1024; // 1MB
 
-  private static final List<String> WRITER_CLS_NAMES =
-    ImmutableList.of(ProtobufLogWriter.class.getSimpleName(),
-      AsyncProtobufLogWriter.class.getSimpleName(), SecureProtobufLogWriter.class.getSimpleName(),
-      SecureAsyncProtobufLogWriter.class.getSimpleName());
+  private static final List<String> WRITER_CLS_NAMES = ImmutableList.of(
+    ProtobufLogWriter.class.getSimpleName(), AsyncProtobufLogWriter.class.getSimpleName(),
+    "SecureProtobufLogWriter", "SecureAsyncProtobufLogWriter");
 
   protected Configuration conf;
 

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

@@ -127,15 +127,6 @@ public AsyncProtobufLogWriter(EventLoopGroup eventLoopGroup,
       AbstractWALRoller.DEFAULT_WAL_ROLL_WAIT_TIMEOUT);
   }
 
-  /*
-   * @return class name which is recognized by hbase-1.x to avoid ProtobufLogReader throwing error:
-   * IOException: Got unknown writer class: AsyncProtobufLogWriter
-   */
-  @Override
-  protected String getWriterClassName() {
-    return "ProtobufLogWriter";
-  }
-
   @Override
   public void append(Entry entry) {
     int buffered = output.buffered();

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

@@ -49,7 +49,7 @@ public class AsyncFSWALProvider extends AbstractFSWALProvider<AsyncFSWAL> {
 
   private static final Logger LOG = LoggerFactory.getLogger(AsyncFSWALProvider.class);
 
-  public static final String WRITER_IMPL = "hbase.regionserver.hlog.async.writer.impl";
+  public static final String WRITER_IMPL = "hbase.regionserver.wal.async.writer.impl";
 
   // Only public so classes back in regionserver.wal can access
   public interface AsyncWriter extends WALProvider.AsyncWriter {

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

@@ -41,6 +41,8 @@ public class FSHLogProvider extends AbstractFSWALProvider<FSHLog> {
 
   private static final Logger LOG = LoggerFactory.getLogger(FSHLogProvider.class);
 
+  public static final String WRITER_IMPL = "hbase.regionserver.wal.writer.impl";
+
   // Only public so classes back in regionserver.wal can access
   public interface Writer extends WALProvider.Writer {
     /**
@@ -71,7 +73,7 @@ public static Writer createWriter(final Configuration conf, final FileSystem fs,
     final boolean overwritable, long blocksize) throws IOException {
     // Configuration already does caching for the Class lookup.
     Class<? extends Writer> logWriterClass =
-      conf.getClass("hbase.regionserver.hlog.writer.impl", ProtobufLogWriter.class, Writer.class);
+      conf.getClass(WRITER_IMPL, ProtobufLogWriter.class, Writer.class);
     Writer writer = null;
     try {
       writer = logWriterClass.getDeclaredConstructor().newInstance();

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

@@ -27,11 +27,6 @@ public InstrumentedLogWriter() {
     super();
   }
 
-  @Override
-  protected String getWriterClassName() {
-    return ProtobufLogWriter.class.getSimpleName();
-  }
-
   public static boolean activateFailure = false;
 
   @Override