You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ec...@apache.org on 2015/08/12 20:59:24 UTC

hbase git commit: HBASE-14201 hbck should not take a lock unless fixing errors

Repository: hbase
Updated Branches:
  refs/heads/branch-1.2 128dcd177 -> 25bf721b5


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 <ec...@apache.org>


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

Branch: refs/heads/branch-1.2
Commit: 25bf721b517b7740821dbffc5f7c7b5c539e9aca
Parents: 128dcd1
Author: Simon Law <sf...@fb.com>
Authored: Thu Aug 6 00:06:48 2015 -0700
Committer: Elliott Clark <ec...@apache.org>
Committed: Wed Aug 12 11:59:04 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/25bf721b/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://> 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/25bf721b/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/25bf721b/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);
     }