hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jmhs...@apache.org
Subject [43/50] [abbrv] hbase git commit: HBASE-13350 Log warnings for sanity check failures when checks disabled.
Date Fri, 01 May 2015 15:28:29 GMT
HBASE-13350 Log warnings for sanity check failures when checks disabled.

Signed-off-by: Matteo Bertozzi <matteo.bertozzi@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/ddab4726
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/ddab4726
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/ddab4726

Branch: refs/heads/hbase-11339
Commit: ddab4726f6675943fc31b697a3d974bab74cd9ec
Parents: 682a29a
Author: Matt Warhaftig <mwarhaftig@gmail.com>
Authored: Wed Apr 15 00:29:34 2015 -0400
Committer: Matteo Bertozzi <matteo.bertozzi@cloudera.com>
Committed: Thu Apr 16 09:51:47 2015 +0100

----------------------------------------------------------------------
 .../org/apache/hadoop/hbase/master/HMaster.java | 64 +++++++++++---------
 .../hadoop/hbase/client/TestFromClientSide.java | 41 +++++++++++++
 2 files changed, 78 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/ddab4726/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
index bcc43f4..9bd1dbb 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -1373,12 +1373,13 @@ public class HMaster extends HRegionServer implements MasterServices,
Server {
    */
   private void sanityCheckTableDescriptor(final HTableDescriptor htd) throws IOException
{
     final String CONF_KEY = "hbase.table.sanity.checks";
+    boolean logWarn = false;
     if (!conf.getBoolean(CONF_KEY, true)) {
-      return;
+      logWarn = true;
     }
     String tableVal = htd.getConfigurationValue(CONF_KEY);
     if (tableVal != null && !Boolean.valueOf(tableVal)) {
-      return;
+      logWarn = true;
     }
 
     // check max file size
@@ -1388,11 +1389,11 @@ public class HMaster extends HRegionServer implements MasterServices,
Server {
       maxFileSize = conf.getLong(HConstants.HREGION_MAX_FILESIZE, maxFileSizeLowerLimit);
     }
     if (maxFileSize < conf.getLong("hbase.hregion.max.filesize.limit", maxFileSizeLowerLimit))
{
-      throw new DoNotRetryIOException("MAX_FILESIZE for table descriptor or "
-        + "\"hbase.hregion.max.filesize\" (" + maxFileSize
-        + ") is too small, which might cause over splitting into unmanageable "
-        + "number of regions. Set " + CONF_KEY + " to false at conf or table descriptor "
-          + "if you want to bypass sanity checks");
+      String message = "MAX_FILESIZE for table descriptor or "
+          + "\"hbase.hregion.max.filesize\" (" + maxFileSize
+          + ") is too small, which might cause over splitting into unmanageable "
+          + "number of regions.";
+      warnOrThrowExceptionForFailure(logWarn, CONF_KEY, message, null);
     }
 
     // check flush size
@@ -1402,72 +1403,81 @@ public class HMaster extends HRegionServer implements MasterServices,
Server {
       flushSize = conf.getLong(HConstants.HREGION_MEMSTORE_FLUSH_SIZE, flushSizeLowerLimit);
     }
     if (flushSize < conf.getLong("hbase.hregion.memstore.flush.size.limit", flushSizeLowerLimit))
{
-      throw new DoNotRetryIOException("MEMSTORE_FLUSHSIZE for table descriptor or "
+      String message = "MEMSTORE_FLUSHSIZE for table descriptor or "
           + "\"hbase.hregion.memstore.flush.size\" ("+flushSize+") is too small, which might
cause"
-          + " very frequent flushing. Set " + CONF_KEY + " to false at conf or table descriptor
"
-          + "if you want to bypass sanity checks");
+          + " very frequent flushing.";
+      warnOrThrowExceptionForFailure(logWarn, CONF_KEY, message, null);
     }
 
     // check that coprocessors and other specified plugin classes can be loaded
     try {
       checkClassLoading(conf, htd);
     } catch (Exception ex) {
-      throw new DoNotRetryIOException(ex);
+      warnOrThrowExceptionForFailure(logWarn, CONF_KEY, ex.getMessage(), null);
     }
 
     // check compression can be loaded
     try {
       checkCompression(htd);
     } catch (IOException e) {
-      throw new DoNotRetryIOException(e.getMessage(), e);
+      warnOrThrowExceptionForFailure(logWarn, CONF_KEY, e.getMessage(), e);
     }
 
     // check encryption can be loaded
     try {
       checkEncryption(conf, htd);
     } catch (IOException e) {
-      throw new DoNotRetryIOException(e.getMessage(), e);
+      warnOrThrowExceptionForFailure(logWarn, CONF_KEY, e.getMessage(), e);
     }
 
     // check that we have at least 1 CF
     if (htd.getColumnFamilies().length == 0) {
-      throw new DoNotRetryIOException("Table should have at least one column family "
-          + "Set "+CONF_KEY+" at conf or table descriptor if you want to bypass sanity checks");
+      String message = "Table should have at least one column family.";
+      warnOrThrowExceptionForFailure(logWarn, CONF_KEY, message, null);
     }
 
     for (HColumnDescriptor hcd : htd.getColumnFamilies()) {
       if (hcd.getTimeToLive() <= 0) {
-        throw new DoNotRetryIOException("TTL for column family " + hcd.getNameAsString()
-          + "  must be positive. Set " + CONF_KEY + " to false at conf or table descriptor
"
-          + "if you want to bypass sanity checks");
+        String message = "TTL for column family " + hcd.getNameAsString() + " must be positive.";
+        warnOrThrowExceptionForFailure(logWarn, CONF_KEY, message, null);
       }
 
       // check blockSize
       if (hcd.getBlocksize() < 1024 || hcd.getBlocksize() > 16 * 1024 * 1024) {
-        throw new DoNotRetryIOException("Block size for column family " + hcd.getNameAsString()
-          + "  must be between 1K and 16MB Set "+CONF_KEY+" to false at conf or table descriptor
"
-          + "if you want to bypass sanity checks");
+        String message = "Block size for column family " + hcd.getNameAsString()
+            + "  must be between 1K and 16MB.";
+        warnOrThrowExceptionForFailure(logWarn, CONF_KEY, message, null);
       }
 
       // check versions
       if (hcd.getMinVersions() < 0) {
-        throw new DoNotRetryIOException("Min versions for column family " + hcd.getNameAsString()
-          + "  must be positive. Set " + CONF_KEY + " to false at conf or table descriptor
"
-          + "if you want to bypass sanity checks");
+        String message = "Min versions for column family " + hcd.getNameAsString()
+          + "  must be positive.";
+        warnOrThrowExceptionForFailure(logWarn, CONF_KEY, message, null);
       }
       // max versions already being checked
 
       // check replication scope
       if (hcd.getScope() < 0) {
-        throw new DoNotRetryIOException("Replication scope for column family "
-          + hcd.getNameAsString() + "  must be positive. Set " + CONF_KEY + " to false at
conf "
-          + "or table descriptor if you want to bypass sanity checks");
+        String message = "Replication scope for column family "
+          + hcd.getNameAsString() + "  must be positive.";
+        warnOrThrowExceptionForFailure(logWarn, CONF_KEY, message, null);
       }
 
       // TODO: should we check coprocessors and encryption ?
     }
   }
 
+  // HBASE-13350 - Helper method to log warning on sanity check failures if checks disabled.
+  private static void warnOrThrowExceptionForFailure(boolean logWarn, String confKey,
+      String message, Exception cause) throws IOException {
+    if (!logWarn) {
+      throw new DoNotRetryIOException(message + " Set " + confKey +
+          " to false at conf or table descriptor if you want to bypass sanity checks", cause);
+    }
+    LOG.warn(message);
+  }
+
   private void startActiveMasterManager(int infoPort) throws KeeperException {
     String backupZNode = ZKUtil.joinZNode(
       zooKeeper.backupMasterAddressesZNode, serverName.toString());

http://git-wip-us.apache.org/repos/asf/hbase/blob/ddab4726/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
index 5dba49c..e337ce2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
@@ -43,6 +43,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.log4j.Level;
 import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -82,6 +83,7 @@ import org.apache.hadoop.hbase.filter.WhileMatchFilter;
 import org.apache.hadoop.hbase.io.hfile.BlockCache;
 import org.apache.hadoop.hbase.io.hfile.CacheConfig;
 import org.apache.hadoop.hbase.ipc.CoprocessorRpcChannel;
+import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.generated.AdminProtos;
 import org.apache.hadoop.hbase.protobuf.generated.ClientProtos.MutationProto;
@@ -99,6 +101,9 @@ import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher;
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.Logger;
+import org.apache.log4j.spi.LoggingEvent;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
@@ -5438,8 +5443,44 @@ public class TestFromClientSide {
 
     // check the conf settings to disable sanity checks
     htd.setMemStoreFlushSize(0);
+
+    // Check that logs warn on invalid table but allow it.
+    ListAppender listAppender = new ListAppender();
+    Logger log = Logger.getLogger(HMaster.class);
+    log.addAppender(listAppender);
+    log.setLevel(Level.WARN);
+
     htd.setConfiguration("hbase.table.sanity.checks", Boolean.FALSE.toString());
     checkTableIsLegal(htd);
+
+    assertFalse(listAppender.getMessages().isEmpty());
+    assertTrue(listAppender.getMessages().get(0).startsWith("MEMSTORE_FLUSHSIZE for table
"
+        + "descriptor or \"hbase.hregion.memstore.flush.size\" (0) is too small, which might
"
+        + "cause very frequent flushing."));
+
+    log.removeAppender(listAppender);
+  }
+
+  private static class ListAppender extends AppenderSkeleton {
+    private final List<String> messages = new ArrayList<String>();
+
+    @Override
+    protected void append(LoggingEvent event) {
+      messages.add(event.getMessage().toString());
+    }
+
+    @Override
+    public void close() {
+    }
+
+    @Override
+    public boolean requiresLayout() {
+      return false;
+    }
+
+    public List<String> getMessages() {
+      return messages;
+    }
   }
 
   private void checkTableIsLegal(HTableDescriptor htd) throws IOException {


Mime
View raw message