You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by bu...@apache.org on 2018/02/26 17:28:27 UTC

[1/6] hbase git commit: HBASE-18025 CatalogJanitor should collect outdated RegionStates from the AM

Repository: hbase
Updated Branches:
  refs/heads/branch-1.2 d2fe2a5a9 -> 41cd37cff
  refs/heads/branch-1.3 1a36db146 -> 5f96c356b


HBASE-18025 CatalogJanitor should collect outdated RegionStates from the AM

(cherry picked from commit aaece0ba5e399f248c8255fe509cdb1a862bf299)


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

Branch: refs/heads/branch-1.3
Commit: 4b44c9025fd21b1aab10579c5c4c169003c983a8
Parents: 1a36db1
Author: Esteban Gutierrez <es...@apache.org>
Authored: Fri Aug 11 12:56:20 2017 -0500
Committer: Sean Busbey <bu...@apache.org>
Committed: Mon Feb 26 09:47:18 2018 -0600

----------------------------------------------------------------------
 .../hadoop/hbase/master/CatalogJanitor.java     |   4 +
 .../hadoop/hbase/master/RegionStates.java       |   6 +
 .../hadoop/hbase/master/ServerManager.java      |   7 +
 .../TestCatalogJanitorInMemoryStates.java       | 188 +++++++++++++++++++
 4 files changed, 205 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/4b44c902/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
index 48105c8..893b108 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
@@ -216,6 +216,8 @@ public class CatalogJanitor extends ScheduledChore {
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, regionA);
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, regionB);
       MetaTableAccessor.deleteMergeQualifiers(services.getConnection(), mergedRegion);
+      services.getAssignmentManager().getRegionStates().deleteRegion(regionA);
+      services.getAssignmentManager().getRegionStates().deleteRegion(regionB);
       services.getServerManager().removeRegion(regionA);
       services.getServerManager().removeRegion(regionB);
       return true;
@@ -350,6 +352,8 @@ public class CatalogJanitor extends ScheduledChore {
       if (LOG.isTraceEnabled()) LOG.trace("Archiving parent region: " + parent);
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, parent);
       MetaTableAccessor.deleteRegion(this.connection, parent);
+      if (services.getAssignmentManager().getRegionStates() != null)
+        services.getAssignmentManager().getRegionStates().deleteRegion(parent);
       services.getServerManager().removeRegion(parent);
       result = true;
     }

http://git-wip-us.apache.org/repos/asf/hbase/blob/4b44c902/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
index 9fda8da..406f5e6 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
@@ -858,6 +858,12 @@ public class RegionStates {
     }
   }
 
+  @VisibleForTesting
+  public boolean isRegionInRegionStates(final HRegionInfo hri) {
+    return (getRegionState(hri) != null || isRegionOnline(hri)) || isRegionInTransition(hri)
+        || isRegionInState(hri, State.OFFLINE, State.CLOSED);
+     }
+
   /**
    * Checking if a region was assigned to a server which is not online now.
    * If so, we should hold re-assign this region till SSH has split its wals.

http://git-wip-us.apache.org/repos/asf/hbase/blob/4b44c902/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
index 7ea5fd7..c814c2b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
@@ -1223,6 +1223,13 @@ public class ServerManager {
     flushedSequenceIdByRegion.remove(encodedName);
   }
 
+  @VisibleForTesting
+  public boolean isRegionInServerManagerStates(final HRegionInfo hri) {
+    final byte[] encodedName = hri.getEncodedNameAsBytes();
+    return (storeFlushedSequenceIdsByRegion.containsKey(encodedName)
+        || flushedSequenceIdByRegion.containsKey(encodedName));
+  }
+
   /**
    * Called by delete table and similar to notify the ServerManager that a region was removed.
    */

http://git-wip-us.apache.org/repos/asf/hbase/blob/4b44c902/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
new file mode 100644
index 0000000..d2bed9b
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
@@ -0,0 +1,188 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.*;
+import org.apache.hadoop.hbase.client.*;
+import org.apache.hadoop.hbase.master.AssignmentManager;
+import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.PairOfSameType;
+import org.apache.hadoop.hbase.util.Threads;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.junit.rules.TestName;
+import org.junit.rules.TestRule;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
+
+@Category({MasterTests.class, MediumTests.class})
+public class TestCatalogJanitorInMemoryStates {
+  private static final Log LOG = LogFactory.getLog(TestCatalogJanitorInMemoryStates.class);
+  @Rule public final TestRule timeout = CategoryBasedTimeout.builder().
+     withTimeout(this.getClass()).withLookingForStuckThread(true).build();
+  @Rule public final TestName name = new TestName();
+  protected final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+  private static byte [] ROW = Bytes.toBytes("testRow");
+  private static byte [] FAMILY = Bytes.toBytes("testFamily");
+  private static byte [] QUALIFIER = Bytes.toBytes("testQualifier");
+  private static byte [] VALUE = Bytes.toBytes("testValue");
+
+  /**
+   * @throws java.lang.Exception
+   */
+  @BeforeClass
+  public static void setUpBeforeClass() throws Exception {
+    Configuration conf = TEST_UTIL.getConfiguration();
+    TEST_UTIL.startMiniCluster(1);
+  }
+
+  /**
+   * @throws java.lang.Exception
+   */
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+    TEST_UTIL.shutdownMiniCluster();
+  }
+
+  /**
+   * Test clearing a split parent from memory.
+   */
+  @Test(timeout = 180000)
+  public void testInMemoryParentCleanup() throws IOException, InterruptedException {
+    final AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager();
+    final ServerManager sm = TEST_UTIL.getHBaseCluster().getMaster().getServerManager();
+    final CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().catalogJanitorChore;
+
+    Admin admin = TEST_UTIL.getHBaseAdmin();
+    admin.enableCatalogJanitor(false);
+
+    final TableName tableName = TableName.valueOf(name.getMethodName());
+    Table t = TEST_UTIL.createTable(tableName, FAMILY);
+    int rowCount = TEST_UTIL.loadTable(t, FAMILY, false);
+
+    RegionLocator locator = TEST_UTIL.getConnection().getRegionLocator(tableName);
+    List<HRegionLocation> allRegionLocations = locator.getAllRegionLocations();
+
+    // We need to create a valid split with daughter regions
+    HRegionLocation parent = allRegionLocations.get(0);
+    List<HRegionLocation> daughters = splitRegion(parent.getRegionInfo());
+    LOG.info("Parent region: " + parent);
+    LOG.info("Daughter regions: " + daughters);
+    assertNotNull("Should have found daughter regions for " + parent, daughters);
+
+    assertTrue("Parent region should exist in RegionStates",
+        am.getRegionStates().isRegionInRegionStates(parent.getRegionInfo()));
+    assertTrue("Parent region should exist in ServerManager",
+        sm.isRegionInServerManagerStates(parent.getRegionInfo()));
+
+    // clean the parent
+    Result r = MetaMockingUtil.getMetaTableRowResult(parent.getRegionInfo(), null,
+        daughters.get(0).getRegionInfo(), daughters.get(1).getRegionInfo());
+    // We need to wait a little before and after we remove the parent
+    Thread.sleep(5000);
+    janitor.cleanParent(parent.getRegionInfo(), r);
+    Thread.sleep(5000);
+    assertFalse("Parent region should have been removed from RegionStates",
+        am.getRegionStates().isRegionInRegionStates(parent.getRegionInfo()));
+    assertFalse("Parent region should have been removed from ServerManager",
+        sm.isRegionInServerManagerStates(parent.getRegionInfo()));
+
+  }
+
+  /*
+ * Splits a region
+ * @param t Region to split.
+ * @return List of region locations
+ * @throws IOException, InterruptedException
+ */
+  private List<HRegionLocation> splitRegion(final HRegionInfo r)
+      throws IOException, InterruptedException {
+    List<HRegionLocation> locations = new ArrayList<>();
+    // Split this table in two.
+    Admin admin = TEST_UTIL.getHBaseAdmin();
+    Connection connection = TEST_UTIL.getConnection();
+    admin.splitRegion(r.getEncodedNameAsBytes());
+    admin.close();
+    PairOfSameType<HRegionInfo> regions = waitOnDaughters(r);
+    if (regions != null) {
+      try (RegionLocator rl = connection.getRegionLocator(r.getTable())) {
+        locations.add(rl.getRegionLocation(regions.getFirst().getEncodedNameAsBytes()));
+        locations.add(rl.getRegionLocation(regions.getSecond().getEncodedNameAsBytes()));
+      }
+      return locations;
+    }
+    return locations;
+  }
+
+  /*
+   * Wait on region split. May return because we waited long enough on the split
+   * and it didn't happen.  Caller should check.
+   * @param r
+   * @return Daughter regions; caller needs to check table actually split.
+   */
+  private PairOfSameType<HRegionInfo> waitOnDaughters(final HRegionInfo r)
+      throws IOException {
+    long start = System.currentTimeMillis();
+    PairOfSameType<HRegionInfo> pair = null;
+    try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration());
+         Table metaTable = conn.getTable(TableName.META_TABLE_NAME)) {
+      Result result = null;
+      HRegionInfo region = null;
+      while ((System.currentTimeMillis() - start) < 60000) {
+        result = metaTable.get(new Get(r.getRegionName()));
+        if (result == null) {
+          break;
+        }
+        region = MetaTableAccessor.getHRegionInfo(result);
+        if (region.isSplitParent()) {
+          LOG.debug(region.toString() + " IS a parent!");
+          pair = MetaTableAccessor.getDaughterRegions(result);
+          break;
+        }
+        Threads.sleep(100);
+      }
+
+      if (pair.getFirst() == null || pair.getSecond() == null) {
+        throw new IOException("Failed to get daughters, for parent region: " + r);
+      }
+      return pair;
+    }
+  }
+}


[4/6] hbase git commit: HBASE-18025 CatalogJanitor should collect outdated RegionStates from the AM

Posted by bu...@apache.org.
HBASE-18025 CatalogJanitor should collect outdated RegionStates from the AM

(cherry picked from commit aaece0ba5e399f248c8255fe509cdb1a862bf299)


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

Branch: refs/heads/branch-1.2
Commit: 688d4dec74d16f7641a347f3dcd7aa4fa686758d
Parents: d2fe2a5
Author: Esteban Gutierrez <es...@apache.org>
Authored: Fri Aug 11 12:56:20 2017 -0500
Committer: Sean Busbey <bu...@apache.org>
Committed: Mon Feb 26 10:46:44 2018 -0600

----------------------------------------------------------------------
 .../hadoop/hbase/master/CatalogJanitor.java     |   4 +
 .../hadoop/hbase/master/RegionStates.java       |   6 +
 .../hadoop/hbase/master/ServerManager.java      |   7 +
 .../TestCatalogJanitorInMemoryStates.java       | 188 +++++++++++++++++++
 4 files changed, 205 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/688d4dec/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
index 48105c8..893b108 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
@@ -216,6 +216,8 @@ public class CatalogJanitor extends ScheduledChore {
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, regionA);
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, regionB);
       MetaTableAccessor.deleteMergeQualifiers(services.getConnection(), mergedRegion);
+      services.getAssignmentManager().getRegionStates().deleteRegion(regionA);
+      services.getAssignmentManager().getRegionStates().deleteRegion(regionB);
       services.getServerManager().removeRegion(regionA);
       services.getServerManager().removeRegion(regionB);
       return true;
@@ -350,6 +352,8 @@ public class CatalogJanitor extends ScheduledChore {
       if (LOG.isTraceEnabled()) LOG.trace("Archiving parent region: " + parent);
       HFileArchiver.archiveRegion(this.services.getConfiguration(), fs, parent);
       MetaTableAccessor.deleteRegion(this.connection, parent);
+      if (services.getAssignmentManager().getRegionStates() != null)
+        services.getAssignmentManager().getRegionStates().deleteRegion(parent);
       services.getServerManager().removeRegion(parent);
       result = true;
     }

http://git-wip-us.apache.org/repos/asf/hbase/blob/688d4dec/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
index bfdfecc..cf68a02 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
@@ -848,6 +848,12 @@ public class RegionStates {
     }
   }
 
+  @VisibleForTesting
+  public boolean isRegionInRegionStates(final HRegionInfo hri) {
+    return (getRegionState(hri) != null || isRegionOnline(hri)) || isRegionInTransition(hri)
+        || isRegionInState(hri, State.OFFLINE, State.CLOSED);
+     }
+
   /**
    * Checking if a region was assigned to a server which is not online now.
    * If so, we should hold re-assign this region till SSH has split its wals.

http://git-wip-us.apache.org/repos/asf/hbase/blob/688d4dec/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
index a420705..7527af7 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
@@ -1222,6 +1222,13 @@ public class ServerManager {
     flushedSequenceIdByRegion.remove(encodedName);
   }
 
+  @VisibleForTesting
+  public boolean isRegionInServerManagerStates(final HRegionInfo hri) {
+    final byte[] encodedName = hri.getEncodedNameAsBytes();
+    return (storeFlushedSequenceIdsByRegion.containsKey(encodedName)
+        || flushedSequenceIdByRegion.containsKey(encodedName));
+  }
+
   /**
    * Called by delete table and similar to notify the ServerManager that a region was removed.
    */

http://git-wip-us.apache.org/repos/asf/hbase/blob/688d4dec/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
new file mode 100644
index 0000000..d2bed9b
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
@@ -0,0 +1,188 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.*;
+import org.apache.hadoop.hbase.client.*;
+import org.apache.hadoop.hbase.master.AssignmentManager;
+import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.PairOfSameType;
+import org.apache.hadoop.hbase.util.Threads;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.junit.rules.TestName;
+import org.junit.rules.TestRule;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
+
+@Category({MasterTests.class, MediumTests.class})
+public class TestCatalogJanitorInMemoryStates {
+  private static final Log LOG = LogFactory.getLog(TestCatalogJanitorInMemoryStates.class);
+  @Rule public final TestRule timeout = CategoryBasedTimeout.builder().
+     withTimeout(this.getClass()).withLookingForStuckThread(true).build();
+  @Rule public final TestName name = new TestName();
+  protected final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+  private static byte [] ROW = Bytes.toBytes("testRow");
+  private static byte [] FAMILY = Bytes.toBytes("testFamily");
+  private static byte [] QUALIFIER = Bytes.toBytes("testQualifier");
+  private static byte [] VALUE = Bytes.toBytes("testValue");
+
+  /**
+   * @throws java.lang.Exception
+   */
+  @BeforeClass
+  public static void setUpBeforeClass() throws Exception {
+    Configuration conf = TEST_UTIL.getConfiguration();
+    TEST_UTIL.startMiniCluster(1);
+  }
+
+  /**
+   * @throws java.lang.Exception
+   */
+  @AfterClass
+  public static void tearDownAfterClass() throws Exception {
+    TEST_UTIL.shutdownMiniCluster();
+  }
+
+  /**
+   * Test clearing a split parent from memory.
+   */
+  @Test(timeout = 180000)
+  public void testInMemoryParentCleanup() throws IOException, InterruptedException {
+    final AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager();
+    final ServerManager sm = TEST_UTIL.getHBaseCluster().getMaster().getServerManager();
+    final CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().catalogJanitorChore;
+
+    Admin admin = TEST_UTIL.getHBaseAdmin();
+    admin.enableCatalogJanitor(false);
+
+    final TableName tableName = TableName.valueOf(name.getMethodName());
+    Table t = TEST_UTIL.createTable(tableName, FAMILY);
+    int rowCount = TEST_UTIL.loadTable(t, FAMILY, false);
+
+    RegionLocator locator = TEST_UTIL.getConnection().getRegionLocator(tableName);
+    List<HRegionLocation> allRegionLocations = locator.getAllRegionLocations();
+
+    // We need to create a valid split with daughter regions
+    HRegionLocation parent = allRegionLocations.get(0);
+    List<HRegionLocation> daughters = splitRegion(parent.getRegionInfo());
+    LOG.info("Parent region: " + parent);
+    LOG.info("Daughter regions: " + daughters);
+    assertNotNull("Should have found daughter regions for " + parent, daughters);
+
+    assertTrue("Parent region should exist in RegionStates",
+        am.getRegionStates().isRegionInRegionStates(parent.getRegionInfo()));
+    assertTrue("Parent region should exist in ServerManager",
+        sm.isRegionInServerManagerStates(parent.getRegionInfo()));
+
+    // clean the parent
+    Result r = MetaMockingUtil.getMetaTableRowResult(parent.getRegionInfo(), null,
+        daughters.get(0).getRegionInfo(), daughters.get(1).getRegionInfo());
+    // We need to wait a little before and after we remove the parent
+    Thread.sleep(5000);
+    janitor.cleanParent(parent.getRegionInfo(), r);
+    Thread.sleep(5000);
+    assertFalse("Parent region should have been removed from RegionStates",
+        am.getRegionStates().isRegionInRegionStates(parent.getRegionInfo()));
+    assertFalse("Parent region should have been removed from ServerManager",
+        sm.isRegionInServerManagerStates(parent.getRegionInfo()));
+
+  }
+
+  /*
+ * Splits a region
+ * @param t Region to split.
+ * @return List of region locations
+ * @throws IOException, InterruptedException
+ */
+  private List<HRegionLocation> splitRegion(final HRegionInfo r)
+      throws IOException, InterruptedException {
+    List<HRegionLocation> locations = new ArrayList<>();
+    // Split this table in two.
+    Admin admin = TEST_UTIL.getHBaseAdmin();
+    Connection connection = TEST_UTIL.getConnection();
+    admin.splitRegion(r.getEncodedNameAsBytes());
+    admin.close();
+    PairOfSameType<HRegionInfo> regions = waitOnDaughters(r);
+    if (regions != null) {
+      try (RegionLocator rl = connection.getRegionLocator(r.getTable())) {
+        locations.add(rl.getRegionLocation(regions.getFirst().getEncodedNameAsBytes()));
+        locations.add(rl.getRegionLocation(regions.getSecond().getEncodedNameAsBytes()));
+      }
+      return locations;
+    }
+    return locations;
+  }
+
+  /*
+   * Wait on region split. May return because we waited long enough on the split
+   * and it didn't happen.  Caller should check.
+   * @param r
+   * @return Daughter regions; caller needs to check table actually split.
+   */
+  private PairOfSameType<HRegionInfo> waitOnDaughters(final HRegionInfo r)
+      throws IOException {
+    long start = System.currentTimeMillis();
+    PairOfSameType<HRegionInfo> pair = null;
+    try (Connection conn = ConnectionFactory.createConnection(TEST_UTIL.getConfiguration());
+         Table metaTable = conn.getTable(TableName.META_TABLE_NAME)) {
+      Result result = null;
+      HRegionInfo region = null;
+      while ((System.currentTimeMillis() - start) < 60000) {
+        result = metaTable.get(new Get(r.getRegionName()));
+        if (result == null) {
+          break;
+        }
+        region = MetaTableAccessor.getHRegionInfo(result);
+        if (region.isSplitParent()) {
+          LOG.debug(region.toString() + " IS a parent!");
+          pair = MetaTableAccessor.getDaughterRegions(result);
+          break;
+        }
+        Threads.sleep(100);
+      }
+
+      if (pair.getFirst() == null || pair.getSecond() == null) {
+        throw new IOException("Failed to get daughters, for parent region: " + r);
+      }
+      return pair;
+    }
+  }
+}


[2/6] hbase git commit: HBASE-18625 Splitting of region with replica, doesn't update region list in serverHolding. A server crash leads to overlap.

Posted by bu...@apache.org.
HBASE-18625 Splitting of region with replica, doesn't update region list in serverHolding. A server crash leads to overlap.

Signed-off-by: ramkrishna.s.vasudevan<ra...@intel.com>
(cherry picked from commit 80d183a707c6d0203da1565f6a7ca3edf4bb2291)


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

Branch: refs/heads/branch-1.3
Commit: 5ed1f4166e7f7afb6e5625f5cecd5e045301c15d
Parents: 4b44c90
Author: huaxiang sun <hu...@apache.org>
Authored: Wed Jan 3 12:24:05 2018 -0800
Committer: Sean Busbey <bu...@apache.org>
Committed: Mon Feb 26 09:47:26 2018 -0600

----------------------------------------------------------------------
 .../hadoop/hbase/master/RegionStates.java       | 33 ++++++-
 .../TestCatalogJanitorInMemoryStates.java       | 91 +++++++++++++++++---
 2 files changed, 112 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/5ed1f416/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
index 406f5e6..3d0c840 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
@@ -555,6 +555,19 @@ public class RegionStates {
   }
 
   /**
+   * Used in some unit tests
+   */
+  @VisibleForTesting
+  synchronized boolean existsInServerHoldings(final ServerName serverName,
+      final HRegionInfo hri) {
+    Set<HRegionInfo> oldRegions = serverHoldings.get(serverName);
+    if (oldRegions != null) {
+      return oldRegions.contains(hri);
+    }
+    return false;
+  }
+
+  /**
    * A dead server's wals have been split so that all the regions
    * used to be open on it can be safely assigned now. Mark them assignable.
    */
@@ -623,8 +636,26 @@ public class RegionStates {
       deleteRegion(hri);
       return;
     }
+
+    /*
+     * One tricky case, if region here is a replica region and its parent is at
+     * SPLIT state, its newState should be same as its parent, not OFFLINE.
+     */
     State newState =
-      expectedState == null ? State.OFFLINE : expectedState;
+        expectedState == null ? State.OFFLINE : expectedState;
+
+    if ((expectedState == null) && !RegionReplicaUtil.isDefaultReplica(hri)) {
+      RegionState primateState = getRegionState(
+          RegionReplicaUtil.getRegionInfoForDefaultReplica(hri));
+      if ((primateState != null) && (primateState.getState() == State.SPLIT)) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Update region " + hri + "to SPLIT, from primary region " +
+              RegionReplicaUtil.getRegionInfoForDefaultReplica(hri));
+        }
+        newState = State.SPLIT;
+      }
+    }
+
     updateRegionState(hri, newState);
     String encodedName = hri.getEncodedName();
     synchronized (this) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/5ed1f416/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
index d2bed9b..6ea7e2f 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
@@ -18,21 +18,20 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.*;
 import org.apache.hadoop.hbase.client.*;
-import org.apache.hadoop.hbase.master.AssignmentManager;
-import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
-import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.PairOfSameType;
 import org.apache.hadoop.hbase.util.Threads;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import org.junit.AfterClass;
@@ -44,12 +43,6 @@ import org.junit.experimental.categories.Category;
 import org.junit.rules.TestName;
 import org.junit.rules.TestRule;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertNotNull;
 
 @Category({MasterTests.class, MediumTests.class})
@@ -126,6 +119,57 @@ public class TestCatalogJanitorInMemoryStates {
 
   }
 
+  /**
+   * Test that after replica parent region is split, the parent replica region is removed from
+   * AM's serverHoldings and
+   */
+  @Test(timeout = 180000)
+  public void testInMemoryForReplicaParentCleanup() throws IOException, InterruptedException {
+    final AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager();
+    final CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().catalogJanitorChore;
+
+    final TableName tableName = TableName.valueOf("testInMemoryForReplicaParentCleanup");
+    HTableDescriptor hdt = TEST_UTIL.createTableDescriptor(tableName.getNameAsString());
+    hdt.setRegionReplication(2);
+    TEST_UTIL.createTable(hdt, new byte[][] { FAMILY }, TEST_UTIL.getConfiguration());
+
+    RegionLocator locator = TEST_UTIL.getConnection().getRegionLocator(tableName);
+    List<HRegionLocation> allRegionLocations = locator.getAllRegionLocations();
+
+    // There are two regions, one for primary, one for the replica.
+    assertTrue(allRegionLocations.size() == 2);
+
+    HRegionLocation replicaParentRegion, primaryParentRegion;
+    if (RegionReplicaUtil.isDefaultReplica(
+        allRegionLocations.get(0).getRegionInfo().getReplicaId())) {
+      primaryParentRegion = allRegionLocations.get(0);
+      replicaParentRegion = allRegionLocations.get(1);
+    } else {
+      primaryParentRegion = allRegionLocations.get(1);
+      replicaParentRegion = allRegionLocations.get(0);
+    }
+
+    List<HRegionLocation> primaryDaughters = splitRegion(primaryParentRegion.getRegionInfo(),
+        Bytes.toBytes("a"));
+
+    // Wait until the replica parent region is offline.
+    while (am.getRegionStates().isRegionOnline(replicaParentRegion.getRegionInfo())) {
+      Thread.sleep(100);
+    }
+
+    assertNotNull("Should have found daughter regions for " + primaryDaughters, primaryDaughters);
+
+    // check that primary parent region is not in AM's serverHoldings
+    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
+        am.getRegionStates().existsInServerHoldings(primaryParentRegion.getServerName(),
+            primaryParentRegion.getRegionInfo()));
+
+    // check that primary parent region is not in AM's serverHoldings
+    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
+        am.getRegionStates().existsInServerHoldings(replicaParentRegion.getServerName(),
+            replicaParentRegion.getRegionInfo()));
+  }
+
   /*
  * Splits a region
  * @param t Region to split.
@@ -152,6 +196,31 @@ public class TestCatalogJanitorInMemoryStates {
   }
 
   /*
+* Splits a region
+* @param t Region to split.
+* @return List of region locations
+* @throws IOException, InterruptedException
+*/
+  private List<HRegionLocation> splitRegion(final HRegionInfo r, final byte[] splitPoint)
+      throws IOException, InterruptedException {
+    List<HRegionLocation> locations = new ArrayList<>();
+    // Split this table in two.
+    Admin admin = TEST_UTIL.getHBaseAdmin();
+    Connection connection = TEST_UTIL.getConnection();
+    admin.splitRegion(r.getEncodedNameAsBytes(), splitPoint);
+    admin.close();
+    PairOfSameType<HRegionInfo> regions = waitOnDaughters(r);
+    if (regions != null) {
+      try (RegionLocator rl = connection.getRegionLocator(r.getTable())) {
+        locations.add(rl.getRegionLocation(regions.getFirst().getEncodedNameAsBytes()));
+        locations.add(rl.getRegionLocation(regions.getSecond().getEncodedNameAsBytes()));
+      }
+      return locations;
+    }
+    return locations;
+  }
+
+  /*
    * Wait on region split. May return because we waited long enough on the split
    * and it didn't happen.  Caller should check.
    * @param r


[3/6] hbase git commit: HBASE-20016 TestCatalogJanitorInMemoryStates#testInMemoryForReplicaParentCleanup is flaky

Posted by bu...@apache.org.
HBASE-20016 TestCatalogJanitorInMemoryStates#testInMemoryForReplicaParentCleanup is flaky

(cherry picked from commit 08b993997404aad7ec8f66b8c2b2083062c9ef1a)


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

Branch: refs/heads/branch-1.3
Commit: 5f96c356b8dddaf20f8df3207117400999097127
Parents: 5ed1f41
Author: Chia-Ping Tsai <ch...@gmail.com>
Authored: Sun Feb 18 21:22:58 2018 +0800
Committer: Sean Busbey <bu...@apache.org>
Committed: Mon Feb 26 09:47:31 2018 -0600

----------------------------------------------------------------------
 .../TestCatalogJanitorInMemoryStates.java       | 52 +++++++++-----------
 1 file changed, 22 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/5f96c356/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
index 6ea7e2f..40396ba 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
@@ -124,7 +124,7 @@ public class TestCatalogJanitorInMemoryStates {
    * AM's serverHoldings and
    */
   @Test(timeout = 180000)
-  public void testInMemoryForReplicaParentCleanup() throws IOException, InterruptedException {
+  public void testInMemoryForReplicaParentCleanup() throws Exception {
     final AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager();
     final CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().catalogJanitorChore;
 
@@ -139,35 +139,27 @@ public class TestCatalogJanitorInMemoryStates {
     // There are two regions, one for primary, one for the replica.
     assertTrue(allRegionLocations.size() == 2);
 
-    HRegionLocation replicaParentRegion, primaryParentRegion;
-    if (RegionReplicaUtil.isDefaultReplica(
-        allRegionLocations.get(0).getRegionInfo().getReplicaId())) {
-      primaryParentRegion = allRegionLocations.get(0);
-      replicaParentRegion = allRegionLocations.get(1);
-    } else {
-      primaryParentRegion = allRegionLocations.get(1);
-      replicaParentRegion = allRegionLocations.get(0);
-    }
-
-    List<HRegionLocation> primaryDaughters = splitRegion(primaryParentRegion.getRegionInfo(),
-        Bytes.toBytes("a"));
-
-    // Wait until the replica parent region is offline.
-    while (am.getRegionStates().isRegionOnline(replicaParentRegion.getRegionInfo())) {
-      Thread.sleep(100);
-    }
-
-    assertNotNull("Should have found daughter regions for " + primaryDaughters, primaryDaughters);
-
-    // check that primary parent region is not in AM's serverHoldings
-    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
-        am.getRegionStates().existsInServerHoldings(primaryParentRegion.getServerName(),
-            primaryParentRegion.getRegionInfo()));
-
-    // check that primary parent region is not in AM's serverHoldings
-    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
-        am.getRegionStates().existsInServerHoldings(replicaParentRegion.getServerName(),
-            replicaParentRegion.getRegionInfo()));
+    final HRegionLocation primaryParentRegion
+      = RegionReplicaUtil.isDefaultReplica(
+      allRegionLocations.get(0).getRegionInfo().getReplicaId()) ? allRegionLocations.get(0)
+        : allRegionLocations.get(1);
+    final HRegionLocation replicaParentRegion
+      = RegionReplicaUtil.isDefaultReplica(
+      allRegionLocations.get(0).getRegionInfo().getReplicaId()) ? allRegionLocations.get(1)
+      : allRegionLocations.get(0);
+
+    assertNotNull("Should have found daughter regions for " + primaryParentRegion,
+      splitRegion(primaryParentRegion.getRegionInfo(), Bytes.toBytes("a")));
+
+    TEST_UTIL.waitFor(60 * 1000, new Waiter.Predicate<Exception>() {
+      @Override
+      public boolean evaluate() throws Exception {
+        return !am.getRegionStates().existsInServerHoldings(primaryParentRegion.getServerName(),
+          primaryParentRegion.getRegionInfo()) &&
+          !am.getRegionStates().existsInServerHoldings(replicaParentRegion.getServerName(),
+            replicaParentRegion.getRegionInfo());
+      }
+    });
   }
 
   /*


[5/6] hbase git commit: HBASE-18625 Splitting of region with replica, doesn't update region list in serverHolding. A server crash leads to overlap.

Posted by bu...@apache.org.
HBASE-18625 Splitting of region with replica, doesn't update region list in serverHolding. A server crash leads to overlap.

Signed-off-by: ramkrishna.s.vasudevan<ra...@intel.com>
(cherry picked from commit 80d183a707c6d0203da1565f6a7ca3edf4bb2291)


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

Branch: refs/heads/branch-1.2
Commit: 4b795553ea5a05d16088db08c9cc4994e354d56c
Parents: 688d4de
Author: huaxiang sun <hu...@apache.org>
Authored: Wed Jan 3 12:24:05 2018 -0800
Committer: Sean Busbey <bu...@apache.org>
Committed: Mon Feb 26 10:46:54 2018 -0600

----------------------------------------------------------------------
 .../hadoop/hbase/master/RegionStates.java       | 33 ++++++-
 .../TestCatalogJanitorInMemoryStates.java       | 91 +++++++++++++++++---
 2 files changed, 112 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/4b795553/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
index cf68a02..3b89f7b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
@@ -545,6 +545,19 @@ public class RegionStates {
   }
 
   /**
+   * Used in some unit tests
+   */
+  @VisibleForTesting
+  synchronized boolean existsInServerHoldings(final ServerName serverName,
+      final HRegionInfo hri) {
+    Set<HRegionInfo> oldRegions = serverHoldings.get(serverName);
+    if (oldRegions != null) {
+      return oldRegions.contains(hri);
+    }
+    return false;
+  }
+
+  /**
    * A dead server's wals have been split so that all the regions
    * used to be open on it can be safely assigned now. Mark them assignable.
    */
@@ -613,8 +626,26 @@ public class RegionStates {
       deleteRegion(hri);
       return;
     }
+
+    /*
+     * One tricky case, if region here is a replica region and its parent is at
+     * SPLIT state, its newState should be same as its parent, not OFFLINE.
+     */
     State newState =
-      expectedState == null ? State.OFFLINE : expectedState;
+        expectedState == null ? State.OFFLINE : expectedState;
+
+    if ((expectedState == null) && !RegionReplicaUtil.isDefaultReplica(hri)) {
+      RegionState primateState = getRegionState(
+          RegionReplicaUtil.getRegionInfoForDefaultReplica(hri));
+      if ((primateState != null) && (primateState.getState() == State.SPLIT)) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Update region " + hri + "to SPLIT, from primary region " +
+              RegionReplicaUtil.getRegionInfoForDefaultReplica(hri));
+        }
+        newState = State.SPLIT;
+      }
+    }
+
     updateRegionState(hri, newState);
     String encodedName = hri.getEncodedName();
     synchronized (this) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/4b795553/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
index d2bed9b..6ea7e2f 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
@@ -18,21 +18,20 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.*;
 import org.apache.hadoop.hbase.client.*;
-import org.apache.hadoop.hbase.master.AssignmentManager;
-import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
-import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.PairOfSameType;
 import org.apache.hadoop.hbase.util.Threads;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import org.junit.AfterClass;
@@ -44,12 +43,6 @@ import org.junit.experimental.categories.Category;
 import org.junit.rules.TestName;
 import org.junit.rules.TestRule;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertNotNull;
 
 @Category({MasterTests.class, MediumTests.class})
@@ -126,6 +119,57 @@ public class TestCatalogJanitorInMemoryStates {
 
   }
 
+  /**
+   * Test that after replica parent region is split, the parent replica region is removed from
+   * AM's serverHoldings and
+   */
+  @Test(timeout = 180000)
+  public void testInMemoryForReplicaParentCleanup() throws IOException, InterruptedException {
+    final AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager();
+    final CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().catalogJanitorChore;
+
+    final TableName tableName = TableName.valueOf("testInMemoryForReplicaParentCleanup");
+    HTableDescriptor hdt = TEST_UTIL.createTableDescriptor(tableName.getNameAsString());
+    hdt.setRegionReplication(2);
+    TEST_UTIL.createTable(hdt, new byte[][] { FAMILY }, TEST_UTIL.getConfiguration());
+
+    RegionLocator locator = TEST_UTIL.getConnection().getRegionLocator(tableName);
+    List<HRegionLocation> allRegionLocations = locator.getAllRegionLocations();
+
+    // There are two regions, one for primary, one for the replica.
+    assertTrue(allRegionLocations.size() == 2);
+
+    HRegionLocation replicaParentRegion, primaryParentRegion;
+    if (RegionReplicaUtil.isDefaultReplica(
+        allRegionLocations.get(0).getRegionInfo().getReplicaId())) {
+      primaryParentRegion = allRegionLocations.get(0);
+      replicaParentRegion = allRegionLocations.get(1);
+    } else {
+      primaryParentRegion = allRegionLocations.get(1);
+      replicaParentRegion = allRegionLocations.get(0);
+    }
+
+    List<HRegionLocation> primaryDaughters = splitRegion(primaryParentRegion.getRegionInfo(),
+        Bytes.toBytes("a"));
+
+    // Wait until the replica parent region is offline.
+    while (am.getRegionStates().isRegionOnline(replicaParentRegion.getRegionInfo())) {
+      Thread.sleep(100);
+    }
+
+    assertNotNull("Should have found daughter regions for " + primaryDaughters, primaryDaughters);
+
+    // check that primary parent region is not in AM's serverHoldings
+    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
+        am.getRegionStates().existsInServerHoldings(primaryParentRegion.getServerName(),
+            primaryParentRegion.getRegionInfo()));
+
+    // check that primary parent region is not in AM's serverHoldings
+    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
+        am.getRegionStates().existsInServerHoldings(replicaParentRegion.getServerName(),
+            replicaParentRegion.getRegionInfo()));
+  }
+
   /*
  * Splits a region
  * @param t Region to split.
@@ -152,6 +196,31 @@ public class TestCatalogJanitorInMemoryStates {
   }
 
   /*
+* Splits a region
+* @param t Region to split.
+* @return List of region locations
+* @throws IOException, InterruptedException
+*/
+  private List<HRegionLocation> splitRegion(final HRegionInfo r, final byte[] splitPoint)
+      throws IOException, InterruptedException {
+    List<HRegionLocation> locations = new ArrayList<>();
+    // Split this table in two.
+    Admin admin = TEST_UTIL.getHBaseAdmin();
+    Connection connection = TEST_UTIL.getConnection();
+    admin.splitRegion(r.getEncodedNameAsBytes(), splitPoint);
+    admin.close();
+    PairOfSameType<HRegionInfo> regions = waitOnDaughters(r);
+    if (regions != null) {
+      try (RegionLocator rl = connection.getRegionLocator(r.getTable())) {
+        locations.add(rl.getRegionLocation(regions.getFirst().getEncodedNameAsBytes()));
+        locations.add(rl.getRegionLocation(regions.getSecond().getEncodedNameAsBytes()));
+      }
+      return locations;
+    }
+    return locations;
+  }
+
+  /*
    * Wait on region split. May return because we waited long enough on the split
    * and it didn't happen.  Caller should check.
    * @param r


[6/6] hbase git commit: HBASE-20016 TestCatalogJanitorInMemoryStates#testInMemoryForReplicaParentCleanup is flaky

Posted by bu...@apache.org.
HBASE-20016 TestCatalogJanitorInMemoryStates#testInMemoryForReplicaParentCleanup is flaky

(cherry picked from commit 08b993997404aad7ec8f66b8c2b2083062c9ef1a)


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

Branch: refs/heads/branch-1.2
Commit: 41cd37cffc690d056edf1ca10d5cbc07d02373c5
Parents: 4b79555
Author: Chia-Ping Tsai <ch...@gmail.com>
Authored: Sun Feb 18 21:22:58 2018 +0800
Committer: Sean Busbey <bu...@apache.org>
Committed: Mon Feb 26 10:46:57 2018 -0600

----------------------------------------------------------------------
 .../TestCatalogJanitorInMemoryStates.java       | 52 +++++++++-----------
 1 file changed, 22 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/41cd37cf/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
index 6ea7e2f..40396ba 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitorInMemoryStates.java
@@ -124,7 +124,7 @@ public class TestCatalogJanitorInMemoryStates {
    * AM's serverHoldings and
    */
   @Test(timeout = 180000)
-  public void testInMemoryForReplicaParentCleanup() throws IOException, InterruptedException {
+  public void testInMemoryForReplicaParentCleanup() throws Exception {
     final AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager();
     final CatalogJanitor janitor = TEST_UTIL.getHBaseCluster().getMaster().catalogJanitorChore;
 
@@ -139,35 +139,27 @@ public class TestCatalogJanitorInMemoryStates {
     // There are two regions, one for primary, one for the replica.
     assertTrue(allRegionLocations.size() == 2);
 
-    HRegionLocation replicaParentRegion, primaryParentRegion;
-    if (RegionReplicaUtil.isDefaultReplica(
-        allRegionLocations.get(0).getRegionInfo().getReplicaId())) {
-      primaryParentRegion = allRegionLocations.get(0);
-      replicaParentRegion = allRegionLocations.get(1);
-    } else {
-      primaryParentRegion = allRegionLocations.get(1);
-      replicaParentRegion = allRegionLocations.get(0);
-    }
-
-    List<HRegionLocation> primaryDaughters = splitRegion(primaryParentRegion.getRegionInfo(),
-        Bytes.toBytes("a"));
-
-    // Wait until the replica parent region is offline.
-    while (am.getRegionStates().isRegionOnline(replicaParentRegion.getRegionInfo())) {
-      Thread.sleep(100);
-    }
-
-    assertNotNull("Should have found daughter regions for " + primaryDaughters, primaryDaughters);
-
-    // check that primary parent region is not in AM's serverHoldings
-    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
-        am.getRegionStates().existsInServerHoldings(primaryParentRegion.getServerName(),
-            primaryParentRegion.getRegionInfo()));
-
-    // check that primary parent region is not in AM's serverHoldings
-    assertFalse("Primary Parent region should have been removed from RegionState's serverHoldings",
-        am.getRegionStates().existsInServerHoldings(replicaParentRegion.getServerName(),
-            replicaParentRegion.getRegionInfo()));
+    final HRegionLocation primaryParentRegion
+      = RegionReplicaUtil.isDefaultReplica(
+      allRegionLocations.get(0).getRegionInfo().getReplicaId()) ? allRegionLocations.get(0)
+        : allRegionLocations.get(1);
+    final HRegionLocation replicaParentRegion
+      = RegionReplicaUtil.isDefaultReplica(
+      allRegionLocations.get(0).getRegionInfo().getReplicaId()) ? allRegionLocations.get(1)
+      : allRegionLocations.get(0);
+
+    assertNotNull("Should have found daughter regions for " + primaryParentRegion,
+      splitRegion(primaryParentRegion.getRegionInfo(), Bytes.toBytes("a")));
+
+    TEST_UTIL.waitFor(60 * 1000, new Waiter.Predicate<Exception>() {
+      @Override
+      public boolean evaluate() throws Exception {
+        return !am.getRegionStates().existsInServerHoldings(primaryParentRegion.getServerName(),
+          primaryParentRegion.getRegionInfo()) &&
+          !am.getRegionStates().existsInServerHoldings(replicaParentRegion.getServerName(),
+            replicaParentRegion.getRegionInfo());
+      }
+    });
   }
 
   /*