HBASE-28948 RegionMover tool fails when table is deleted (#6423)

Signed-off-by: Wellington Chevreuil <[email protected]>
(cherry picked from commit 344bf78)

Author: petersomogyi

hbase-server/src/main/java/org/apache/hadoop/hbase/util/MoveWithAck.java

@@ -138,7 +138,7 @@ private boolean isSameServer(RegionInfo region, ServerName serverName) throws IO
    */
   static ServerName getServerNameForRegion(RegionInfo region, Admin admin, Connection conn)
     throws IOException {
-    if (!admin.isTableEnabled(region.getTable())) {
+    if (!admin.tableExists(region.getTable()) || !admin.isTableEnabled(region.getTable())) {
       return null;
     }
     HRegionLocation loc = conn.getRegionLocator(region.getTable())

hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMover2.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.util;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -65,12 +66,12 @@ public class TestRegionMover2 {
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
     HBaseClassTestRule.forClass(TestRegionMover2.class);
+  private static final String CF = "fam1";
 
   @Rule
   public TestName name = new TestName();
 
   private static final Logger LOG = LoggerFactory.getLogger(TestRegionMover2.class);
-
   private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
 
   @BeforeClass
@@ -86,12 +87,7 @@ public static void tearDownAfterClass() throws Exception {
 
   @Before
   public void setUp() throws Exception {
-    final TableName tableName = TableName.valueOf(name.getMethodName());
-    TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(tableName)
-      .setColumnFamily(ColumnFamilyDescriptorBuilder.of("fam1")).build();
-    int startKey = 0;
-    int endKey = 80000;
-    TEST_UTIL.getAdmin().createTable(tableDesc, Bytes.toBytes(startKey), Bytes.toBytes(endKey), 9);
+    createTable(name.getMethodName());
   }
 
   @After
@@ -101,17 +97,23 @@ public void tearDown() throws Exception {
     TEST_UTIL.getAdmin().deleteTable(tableName);
   }
 
+  private TableName createTable(String name) throws IOException {
+    final TableName tableName = TableName.valueOf(name);
+    TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(tableName)
+      .setColumnFamily(ColumnFamilyDescriptorBuilder.of(CF)).build();
+    int startKey = 0;
+    int endKey = 80000;
+    TEST_UTIL.getAdmin().createTable(tableDesc, Bytes.toBytes(startKey), Bytes.toBytes(endKey), 9);
+    return tableName;
+  }
+
   @Test
   public void testWithMergedRegions() throws Exception {
     final TableName tableName = TableName.valueOf(name.getMethodName());
     SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
     Admin admin = TEST_UTIL.getAdmin();
     Table table = TEST_UTIL.getConnection().getTable(tableName);
-    List<Put> puts = new ArrayList<>();
-    for (int i = 0; i < 10000; i++) {
-      puts.add(new Put(Bytes.toBytes("rowkey_" + i)).addColumn(Bytes.toBytes("fam1"),
-        Bytes.toBytes("q1"), Bytes.toBytes("val_" + i)));
-    }
+    List<Put> puts = createPuts(10000);
     table.put(puts);
     admin.flush(tableName);
     HRegionServer regionServer = cluster.getRegionServer(0);
@@ -141,11 +143,7 @@ public void testWithSplitRegions() throws Exception {
     SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
     Admin admin = TEST_UTIL.getAdmin();
     Table table = TEST_UTIL.getConnection().getTable(tableName);
-    List<Put> puts = new ArrayList<>();
-    for (int i = 10; i < 50000; i++) {
-      puts.add(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("fam1"), Bytes.toBytes("q1"),
-        Bytes.toBytes("val_" + i)));
-    }
+    List<Put> puts = createPuts(50000);
     table.put(puts);
     admin.flush(tableName);
     admin.compact(tableName);
@@ -190,16 +188,11 @@ public void testFailedRegionMove() throws Exception {
     SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
     Admin admin = TEST_UTIL.getAdmin();
     Table table = TEST_UTIL.getConnection().getTable(tableName);
-    List<Put> puts = new ArrayList<>();
-    for (int i = 0; i < 1000; i++) {
-      puts.add(new Put(Bytes.toBytes("rowkey_" + i)).addColumn(Bytes.toBytes("fam1"),
-        Bytes.toBytes("q1"), Bytes.toBytes("val_" + i)));
-    }
+    List<Put> puts = createPuts(1000);
     table.put(puts);
     admin.flush(tableName);
     HRegionServer regionServer = cluster.getRegionServer(0);
     String rsName = regionServer.getServerName().getAddress().toString();
-    int numRegions = regionServer.getNumberOfOnlineRegions();
     List<HRegion> hRegions = regionServer.getRegions().stream()
       .filter(hRegion -> hRegion.getRegionInfo().getTable().equals(tableName))
       .collect(Collectors.toList());
@@ -217,14 +210,30 @@ public void testFailedRegionMove() throws Exception {
     }
   }
 
+  @Test
+  public void testDeletedTable() throws Exception {
+    TableName tableNameToDelete = createTable(name.getMethodName() + "ToDelete");
+    SingleProcessHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
+    HRegionServer regionServer = cluster.getRegionServer(0);
+    String rsName = regionServer.getServerName().getAddress().toString();
+    RegionMover.RegionMoverBuilder rmBuilder =
+      new RegionMover.RegionMoverBuilder(rsName, TEST_UTIL.getConfiguration()).ack(true)
+        .maxthreads(8);
+    try (Admin admin = TEST_UTIL.getAdmin(); RegionMover rm = rmBuilder.build()) {
+      LOG.debug("Unloading {}", regionServer.getServerName());
+      rm.unload();
+      Assert.assertEquals(0, regionServer.getNumberOfOnlineRegions());
+      LOG.debug("Successfully Unloaded, now delete table");
+      admin.disableTable(tableNameToDelete);
+      admin.deleteTable(tableNameToDelete);
+      Assert.assertTrue(rm.load());
+    }
+  }
+
   public void loadDummyDataInTable(TableName tableName) throws Exception {
     Admin admin = TEST_UTIL.getAdmin();
     Table table = TEST_UTIL.getConnection().getTable(tableName);
-    List<Put> puts = new ArrayList<>();
-    for (int i = 0; i < 1000; i++) {
-      puts.add(new Put(Bytes.toBytes("rowkey_" + i)).addColumn(Bytes.toBytes("fam1"),
-        Bytes.toBytes("q1"), Bytes.toBytes("val_" + i)));
-    }
+    List<Put> puts = createPuts(1000);
     table.put(puts);
     admin.flush(tableName);
   }
@@ -297,6 +306,15 @@ public void testIsolateMetaAndRandomRegionOnTheRandomServer() throws Exception {
     regionIsolationOperation(randomSeverRegion, randomSeverRegion, 2, true);
   }
 
+  private List<Put> createPuts(int count) {
+    List<Put> puts = new ArrayList<>();
+    for (int i = 0; i < count; i++) {
+      puts.add(new Put(Bytes.toBytes("rowkey_" + i)).addColumn(Bytes.toBytes(CF),
+        Bytes.toBytes("q1"), Bytes.toBytes("val_" + i)));
+    }
+    return puts;
+  }
+
   public ServerName findMetaRSLocation() throws Exception {
     ZKWatcher zkWatcher = new ZKWatcher(TEST_UTIL.getConfiguration(), null, null);
     List<HRegionLocation> result = new ArrayList<>();
@@ -325,7 +343,7 @@ public ServerName findSourceServerName(TableName tableName) throws Exception {
     }
     if (sourceServer == null) {
       throw new Exception(
-        "This shouln't happen, No RS found with more than 2 regions of table : " + tableName);
+        "This shouldn't happen, No RS found with more than 2 regions of table : " + tableName);
     }
     return sourceServer;
   }
@@ -372,7 +390,7 @@ public void regionIsolationOperation(ServerName sourceServerName,
       new RegionMover.RegionMoverBuilder(destinationRSName, TEST_UTIL.getConfiguration()).ack(true)
         .maxthreads(8).isolateRegionIdArray(listOfRegionIDsToIsolate);
     try (RegionMover rm = rmBuilder.build()) {
-      LOG.debug("Unloading {} except regions : {}", destinationRS.getServerName(),
+      LOG.debug("Unloading {} except regions: {}", destinationRS.getServerName(),
         listOfRegionIDsToIsolate);
       rm.isolateRegions();
       Assert.assertEquals(numRegionsToIsolate, destinationRS.getNumberOfOnlineRegions());
@@ -381,8 +399,8 @@ public void regionIsolationOperation(ServerName sourceServerName,
         Assert.assertTrue(
           listOfRegionIDsToIsolate.contains(onlineRegions.get(i).getRegionInfo().getEncodedName()));
       }
-      LOG.debug("Successfully Isolated " + listOfRegionIDsToIsolate.size() + " regions : "
-        + listOfRegionIDsToIsolate + " on " + destinationRS.getServerName());
+      LOG.debug("Successfully Isolated {} regions: {} on {}", listOfRegionIDsToIsolate.size(),
+        listOfRegionIDsToIsolate, destinationRS.getServerName());
     } finally {
       admin.recommissionRegionServer(destinationRS.getServerName(), null);
     }