Return-Path: X-Original-To: apmail-hbase-commits-archive@www.apache.org Delivered-To: apmail-hbase-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id DA6EF18531 for ; Wed, 12 Aug 2015 18:59:42 +0000 (UTC) Received: (qmail 58872 invoked by uid 500); 12 Aug 2015 18:59:36 -0000 Delivered-To: apmail-hbase-commits-archive@hbase.apache.org Received: (qmail 58830 invoked by uid 500); 12 Aug 2015 18:59:36 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 58821 invoked by uid 99); 12 Aug 2015 18:59:36 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 12 Aug 2015 18:59:36 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 606F2E00E0; Wed, 12 Aug 2015 18:59:36 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: eclark@apache.org To: commits@hbase.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: hbase git commit: HBASE-14201 hbck should not take a lock unless fixing errors Date: Wed, 12 Aug 2015 18:59:36 +0000 (UTC) Repository: hbase Updated Branches: refs/heads/branch-1 d50c55d9d -> 881d2c1d7 HBASE-14201 hbck should not take a lock unless fixing errors By default, hbck is run in a read-only checker mode. In this case, it is sensible to let others run. By default, the balancer is left alone, which may cause spurious errors, but cannot leave the balancer in a bad state. It is dangerous to leave the balancer by accident, so it is only ever enabled after fixing, it will never be forced off because of racing. When hbck is run in fixer mode, it must take an exclusive lock and disable the balancer, or all havoc will break loose. If you want to stop hbck from running in parallel, the -exclusive flag will create the lock file. If you want to force -disableBalancer, that option is available too. This makes more semantic sense than -noLock and -noSwitchBalancer, respectively. Signed-off-by: Elliott Clark Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/881d2c1d Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/881d2c1d Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/881d2c1d Branch: refs/heads/branch-1 Commit: 881d2c1d74419013c10a37789f6203ed0bd05dd5 Parents: d50c55d Author: Simon Law Authored: Thu Aug 6 00:06:48 2015 -0700 Committer: Elliott Clark Committed: Wed Aug 12 11:51:27 2015 -0700 ---------------------------------------------------------------------- .../org/apache/hadoop/hbase/util/HBaseFsck.java | 66 +++++++++++++------- .../apache/hadoop/hbase/util/TestHBaseFsck.java | 2 +- .../hadoop/hbase/util/hbck/HbckTestingUtil.java | 2 +- 3 files changed, 47 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/881d2c1d/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java index 7b54371..125128d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java @@ -235,9 +235,9 @@ public class HBaseFsck extends Configured implements Closeable { * Options ***********/ private static boolean details = false; // do we display the full report - private static boolean useLock = true; // do we use the hbck exclusivity lock - private static boolean switchBalancer = true; // do we turn the balancer off while running private long timelag = DEFAULT_TIME_LAG; // tables whose modtime is older + private static boolean forceExclusive = false; // only this hbck can modify HBase + private static boolean disableBalancer = false; // disable load balancer to keep regions stable private boolean fixAssignments = false; // fix assignment errors? private boolean fixMeta = false; // fix meta errors? private boolean checkHdfs = true; // load and check fs consistency? @@ -455,7 +455,7 @@ public class HBaseFsck extends Configured implements Closeable { } private void unlockHbck() { - if (hbckLockCleanup.compareAndSet(true, false)) { + if (isExclusive() && hbckLockCleanup.compareAndSet(true, false)) { RetryCounter retryCounter = lockFileRetryCounterFactory.create(); do { try { @@ -488,13 +488,13 @@ public class HBaseFsck extends Configured implements Closeable { */ public void connect() throws IOException { - if (useLock) { - // Check if another instance of balancer is running + if (isExclusive()) { + // Grab the lock hbckOutFd = checkAndMarkRunningHbck(); if (hbckOutFd == null) { setRetCode(-1); - LOG.error("Another instance of hbck is running, exiting this instance.[If you are sure" + - " no other instance is running, delete the lock file " + + LOG.error("Another instance of hbck is fixing HBase, exiting this instance. " + + "[If you are sure no other instance is running, delete the lock file " + HBCK_LOCK_PATH + " and rerun the tool]"); throw new IOException("Duplicate hbck - Abort"); } @@ -698,9 +698,8 @@ public class HBaseFsck extends Configured implements Closeable { errors.print("Version: " + status.getHBaseVersion()); offlineHdfsIntegrityRepair(); - boolean oldBalancer = true; - // turn the balancer off - if (switchBalancer) { + boolean oldBalancer = false; + if (shouldDisableBalancer()) { oldBalancer = admin.setBalancerRunning(false, true); } @@ -708,7 +707,10 @@ public class HBaseFsck extends Configured implements Closeable { onlineConsistencyRepair(); } finally { - if (switchBalancer) { + // Only restore the balancer if it was true when we started repairing and + // we actually disabled it. Otherwise, we might clobber another run of + // hbck that has just restored it. + if (shouldDisableBalancer() && oldBalancer) { admin.setBalancerRunning(oldBalancer, false); } } @@ -4166,12 +4168,34 @@ public class HBaseFsck extends Configured implements Closeable { details = true; } - public static void setNoLock() { - useLock = false; + /** + * Set exclusive mode. + */ + public static void setForceExclusive() { + forceExclusive = true; + } + + /** + * Only one instance of hbck can modify HBase at a time. + */ + public boolean isExclusive() { + return fixAny || forceExclusive; } - public static void setNoBalacerSwitch() { - switchBalancer = false; + /** + * Disable the load balancer. + */ + public static void setDisableBalancer() { + disableBalancer = true; + } + + /** + * The balancer should be disabled if we are modifying HBase. + * It can be disabled if you want to prevent region movement from causing + * false positives. + */ + public boolean shouldDisableBalancer() { + return fixAny || disableBalancer; } /** @@ -4435,8 +4459,8 @@ public class HBaseFsck extends Configured implements Closeable { out.println(" -metaonly Only check the state of the hbase:meta table."); out.println(" -sidelineDir HDFS path to backup existing meta."); out.println(" -boundaries Verify that regions boundaries are the same between META and store files."); - out.println(" -noLock Turn off using the hdfs lock file."); - out.println(" -noBalancerSwitch Don't switch the balancer off."); + out.println(" -exclusive Abort if another hbck is exclusive or fixing."); + out.println(" -disableBalancer Disable the load balancer."); out.println(""); out.println(" Metadata Repair options: (expert features, use with caution!)"); @@ -4531,10 +4555,10 @@ public class HBaseFsck extends Configured implements Closeable { return printUsageAndExit(); } else if (cmd.equals("-details")) { setDisplayFullReport(); - } else if (cmd.equals("-noLock")) { - setNoLock(); - } else if (cmd.equals("-noBalancerSwitch")) { - setNoBalacerSwitch(); + } else if (cmd.equals("-exclusive")) { + setForceExclusive(); + } else if (cmd.equals("-disableBalancer")) { + setDisableBalancer(); } else if (cmd.equals("-timelag")) { if (i == args.length - 1) { errors.reportError(ERROR_CODE.WRONG_USAGE, "HBaseFsck: -timelag needs a value."); http://git-wip-us.apache.org/repos/asf/hbase/blob/881d2c1d/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java index 4058534..111bf44 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java @@ -587,7 +587,7 @@ public class TestHBaseFsck { // To avoid flakiness of the test, set low max wait time. c.setInt("hbase.hbck.lockfile.maxwaittime", 3); try{ - return doFsck(c, false); + return doFsck(c, true); // Exclusive hbck only when fixing } catch(Exception e){ if (e.getMessage().contains("Duplicate hbck")) { fail = false; http://git-wip-us.apache.org/repos/asf/hbase/blob/881d2c1d/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java index 1577191..013841d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/HbckTestingUtil.java @@ -50,7 +50,6 @@ public class HbckTestingUtil { boolean fixTableZnodes, TableName table) throws Exception { HBaseFsck fsck = new HBaseFsck(conf, exec); - fsck.connect(); fsck.setDisplayFullReport(); // i.e. -details fsck.setTimeLag(0); fsck.setFixAssignments(fixAssignments); @@ -64,6 +63,7 @@ public class HbckTestingUtil { fsck.setFixEmptyMetaCells(fixEmptyMetaRegionInfo); fsck.setFixTableLocks(fixTableLocks); fsck.setFixTableZNodes(fixTableZnodes); + fsck.connect(); if (table != null) { fsck.includeTable(table); }