You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by mb...@apache.org on 2015/04/18 11:26:09 UTC
[06/19] hbase git commit: HBASE-13350 Log warnings for sanity check
failures when checks disabled.
HBASE-13350 Log warnings for sanity check failures when checks disabled.
Signed-off-by: Matteo Bertozzi <ma...@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-12439
Commit: ddab4726f6675943fc31b697a3d974bab74cd9ec
Parents: 682a29a
Author: Matt Warhaftig <mw...@gmail.com>
Authored: Wed Apr 15 00:29:34 2015 -0400
Committer: Matteo Bertozzi <ma...@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 {