From b8f999bf33678e6daa7ce8072ae40824c5ea25d1 Mon Sep 17 00:00:00 2001
From: Sakthi <jatsakthi@cloudera.com>
Date: Thu, 22 Mar 2018 15:43:17 -0700
Subject: [PATCH] HBASE-20135 Fixed NullPointerException during reading bloom
 filter when upgraded from hbase-1 to hbase-2

---
 .../hbase/io/hfile/FixedFileTrailer.java      | 12 ++++++-
 .../hbase/io/hfile/TestFixedFileTrailer.java  | 35 +++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/FixedFileTrailer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/FixedFileTrailer.java
index 55b2ee05f1e..3c74d114e1e 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/FixedFileTrailer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/FixedFileTrailer.java
@@ -38,6 +38,8 @@ import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.HFileProtos;
 import org.apache.hadoop.hbase.util.Bytes;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * The {@link HFile} has a fixed trailer which contains offsets to other
@@ -53,6 +55,8 @@ import org.apache.hadoop.hbase.util.Bytes;
  */
 @InterfaceAudience.Private
 public class FixedFileTrailer {
+  private static final Logger LOG = LoggerFactory.getLogger(FixedFileTrailer.class);
+
   /**
    * We store the comparator class name as a fixed-length field in the trailer.
    */
@@ -623,7 +627,13 @@ public class FixedFileTrailer {
   public static CellComparator createComparator(
       String comparatorClassName) throws IOException {
     try {
-      return getComparatorClass(comparatorClassName).getDeclaredConstructor().newInstance();
+
+      Class<? extends CellComparator> comparatorClass = getComparatorClass(comparatorClassName);
+      if(comparatorClass != null){
+        return comparatorClass.getDeclaredConstructor().newInstance();
+      }
+      LOG.warn("No Comparator class for " + comparatorClassName + ". Returning Null.");
+      return null;
     } catch (Exception e) {
       throw new IOException("Comparator class " + comparatorClassName +
         " is not instantiable", e);
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestFixedFileTrailer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestFixedFileTrailer.java
index 9643ac7ba2a..d25ce4796d7 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestFixedFileTrailer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestFixedFileTrailer.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.io.hfile;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -33,6 +34,7 @@ import org.apache.hadoop.fs.FSDataInputStream;
 import org.apache.hadoop.fs.FSDataOutputStream;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
 import org.apache.hadoop.hbase.CellComparatorImpl;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
@@ -43,8 +45,10 @@ import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.Before;
 import org.junit.ClassRule;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
@@ -82,6 +86,9 @@ public class TestFixedFileTrailer {
     this.version = version;
   }
 
+  @Rule
+  public ExpectedException expectedEx = ExpectedException.none();
+
   @Parameters
   public static Collection<Object[]> getParameters() {
     List<Object[]> versionsToTest = new ArrayList<>();
@@ -108,6 +115,34 @@ public class TestFixedFileTrailer {
         pb.getComparatorClassName());
   }
 
+  @Test
+  public void testCreateComparator() throws IOException {
+    FixedFileTrailer t = new FixedFileTrailer(version, HFileReaderImpl.PBUF_TRAILER_MINOR_VERSION);
+    try {
+      assertEquals(CellComparatorImpl.class,
+          t.createComparator(KeyValue.COMPARATOR.getLegacyKeyComparatorName()).getClass());
+      assertEquals(CellComparatorImpl.class,
+          t.createComparator(KeyValue.COMPARATOR.getClass().getName()).getClass());
+      assertEquals(CellComparatorImpl.class,
+          t.createComparator(CellComparator.class.getName()).getClass());
+      assertEquals(CellComparatorImpl.MetaCellComparator.class,
+          t.createComparator(KeyValue.META_COMPARATOR.getLegacyKeyComparatorName()).getClass());
+      assertEquals(CellComparatorImpl.MetaCellComparator.class,
+          t.createComparator(KeyValue.META_COMPARATOR.getClass().getName()).getClass());
+      assertEquals(CellComparatorImpl.MetaCellComparator.class, t.createComparator(
+          CellComparatorImpl.MetaCellComparator.META_COMPARATOR.getClass().getName()).getClass());
+      assertNull(t.createComparator(Bytes.BYTES_RAWCOMPARATOR.getClass().getName()));
+      assertNull(t.createComparator("org.apache.hadoop.hbase.KeyValue$RawBytesComparator"));
+    } catch (IOException e) {
+      fail("Unexpected exception while testing FixedFileTrailer#createComparator()");
+    }
+
+    // Test an invalid comparatorClassName
+    expectedEx.expect(IOException.class);
+    t.createComparator("");
+
+  }
+
   @Test
   public void testTrailer() throws IOException {
     FixedFileTrailer t = new FixedFileTrailer(version,