HBASE-13350 Log warnings for sanity check failures when checks disabled.
Signed-off-by: Matteo Bertozzi <matteo.bertozzi@cloudera.com>
This commit is contained in:
parent
682a29a57f
commit
ddab4726f6
|
@ -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());
|
||||
|
|
|
@ -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 {
|
||||
|
|
Loading…
Reference in New Issue