From 2b897484327a0afb1ad56101b43b811c851da403 Mon Sep 17 00:00:00 2001
From: Umesh Agashe <uagashe@cloudera.com>
Date: Wed, 8 Mar 2017 13:25:28 -0800
Subject: [PATCH] HBASE-17761: Test TestRemoveRegionMetrics.testMoveRegion
 fails intermittently because of race condition

Signed-off-by: Esteban Gutierrez <esteban@apache.org>
---
 .../hadoop/hbase/HBaseTestingUtility.java     | 24 +++++++++++++++++++
 .../regionserver/TestRemoveRegionMetrics.java |  5 +---
 .../wal/AbstractTestWALReplay.java            | 23 ++----------------
 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
index 47170b16880..a6c7f68a2a4 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
@@ -3297,6 +3297,30 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility {
     return am.waitForAssignment(regionInfo);
   }
 
+  /**
+   * Move region to destination server and wait till region is completely moved and online
+   *
+   * @param destRegion region to move
+   * @param destServer destination server of the region
+   * @throws InterruptedException
+   * @throws IOException
+   */
+  public void moveRegionAndWait(HRegionInfo destRegion, ServerName destServer)
+      throws InterruptedException, IOException {
+    HMaster master = getMiniHBaseCluster().getMaster();
+    getHBaseAdmin().move(destRegion.getEncodedNameAsBytes(),
+        Bytes.toBytes(destServer.getServerName()));
+    while (true) {
+      ServerName serverName = master.getAssignmentManager().getRegionStates()
+          .getRegionServerOfRegion(destRegion);
+      if (serverName != null && serverName.equals(destServer)) {
+        assertRegionOnServer(destRegion, serverName, 200);
+        break;
+      }
+      Thread.sleep(10);
+    }
+  }
+
   /**
    * Wait until all regions for a table in hbase:meta have a non-empty
    * info:server, up to a configuable timeout value (default is 60 seconds)
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRemoveRegionMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRemoveRegionMetrics.java
index eb7a1a1bc26..85699eff8a6 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRemoveRegionMetrics.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRemoveRegionMetrics.java
@@ -96,7 +96,6 @@ public class TestRemoveRegionMetrics {
       int destServerIdx = (currentServerIdx +1)% cluster.getLiveRegionServerThreads().size();
       HRegionServer currentServer = cluster.getRegionServer(currentServerIdx);
       HRegionServer destServer = cluster.getRegionServer(destServerIdx);
-      byte[] destServerName = Bytes.toBytes(destServer.getServerName().getServerName());
 
 
       // Do a put. The counters should be non-zero now
@@ -119,13 +118,11 @@ public class TestRemoveRegionMetrics {
 
 
       try {
-        admin.move(regionInfo.getEncodedNameAsBytes(), destServerName);
+        TEST_UTIL.moveRegionAndWait(regionInfo, destServer.getServerName());
         moved = true;
-        Thread.sleep(5000);
       } catch (IOException ioe) {
         moved = false;
       }
-      TEST_UTIL.waitUntilAllRegionsAssigned(t.getName());
 
       if (moved) {
         MetricsRegionAggregateSource destAgg = destServer.getRegion(regionInfo.getRegionName())
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestWALReplay.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestWALReplay.java
index 237d24a2678..e5e7c8370d1 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestWALReplay.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestWALReplay.java
@@ -218,7 +218,7 @@ public abstract class AbstractTestWALReplay {
     HRegionServer originServer = hbaseCluster.getRegionServer(originServerNum);
     HRegionServer destServer = hbaseCluster.getRegionServer(destServerNum);
     // move region to destination regionserver
-    moveRegionAndWait(destRegion, destServer);
+    TEST_UTIL.moveRegionAndWait(destRegion.getRegionInfo(), destServer.getServerName());
 
     // delete the row
     Delete del = new Delete(Bytes.toBytes("r1"));
@@ -241,7 +241,7 @@ public abstract class AbstractTestWALReplay {
     region.compact(true);
 
     // move region to origin regionserver
-    moveRegionAndWait(destRegion, originServer);
+    TEST_UTIL.moveRegionAndWait(destRegion.getRegionInfo(), originServer.getServerName());
     // abort the origin regionserver
     originServer.abort("testing");
 
@@ -254,25 +254,6 @@ public abstract class AbstractTestWALReplay {
     resultScanner.close();
   }
 
-  private void moveRegionAndWait(Region destRegion, HRegionServer destServer)
-      throws InterruptedException, MasterNotRunningException,
-      ZooKeeperConnectionException, IOException {
-    HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
-    TEST_UTIL.getAdmin().move(
-        destRegion.getRegionInfo().getEncodedNameAsBytes(),
-        Bytes.toBytes(destServer.getServerName().getServerName()));
-    while (true) {
-      ServerName serverName = master.getAssignmentManager()
-        .getRegionStates().getRegionServerOfRegion(destRegion.getRegionInfo());
-      if (serverName != null && serverName.equals(destServer.getServerName())) {
-        TEST_UTIL.assertRegionOnServer(
-          destRegion.getRegionInfo(), serverName, 200);
-        break;
-      }
-      Thread.sleep(10);
-    }
-  }
-
   /**
    * Tests for hbase-2727.
    * @throws Exception