[GitHub] [hbase] HorizonNet commented on a change in pull request #1719: HBASE-24370 Avoid aggressive MergeRegion and GCMultipleMergedRegionsP…

[GitBox <gi...@apache.org>]

HorizonNet commented on a change in pull request #1719:
URL: https://github.com/apache/hbase/pull/1719#discussion_r426242742



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
##########
@@ -161,20 +163,39 @@ public void testMergeRegions() throws Exception {
       .getTableHRegionLocations(metaTable, tableName).get();
     RegionInfo regionA;
     RegionInfo regionB;
+    RegionInfo regionC;
+    RegionInfo mergedChildRegion = null;
 
     // merge with full name
     assertEquals(3, regionLocations.size());
     regionA = regionLocations.get(0).getRegion();
     regionB = regionLocations.get(1).getRegion();
+    regionC = regionLocations.get(2).getRegion();
     admin.mergeRegions(regionA.getRegionName(), regionB.getRegionName(), false).get();
 
     regionLocations = AsyncMetaTableAccessor
       .getTableHRegionLocations(metaTable, tableName).get();
+
     assertEquals(2, regionLocations.size());
+    for (HRegionLocation rl : regionLocations) {
+      if (regionC.compareTo(rl.getRegion()) != 0) {
+        mergedChildRegion = rl.getRegion();
+        break;
+      }
+    }
+
+    assertTrue(mergedChildRegion != null);

Review comment:
       Ditto

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
##########
@@ -530,25 +532,42 @@ public void testMergeRegions() throws Exception {
       List<RegionInfo> tableRegions;
       RegionInfo regionA;
       RegionInfo regionB;
+      RegionInfo regionC;
+      RegionInfo mergedChildRegion = null;
 
       // merge with full name
       tableRegions = ADMIN.getRegions(tableName);
       assertEquals(3, ADMIN.getRegions(tableName).size());
       regionA = tableRegions.get(0);
       regionB = tableRegions.get(1);
+      regionC = tableRegions.get(2);
       // TODO convert this to version that is synchronous (See HBASE-16668)
-      ADMIN.mergeRegionsAsync(regionA.getRegionName(), regionB.getRegionName(), false).get(60,
-        TimeUnit.SECONDS);
+      ADMIN.mergeRegionsAsync(regionA.getRegionName(), regionB.getRegionName(),
+        false).get(60, TimeUnit.SECONDS);
 
-      assertEquals(2, ADMIN.getRegions(tableName).size());
-
-      // merge with encoded name
       tableRegions = ADMIN.getRegions(tableName);
-      regionA = tableRegions.get(0);
-      regionB = tableRegions.get(1);
+
+      assertEquals(2, tableRegions.size());
+      for (RegionInfo ri : tableRegions) {
+        if (regionC.compareTo(ri) != 0) {
+          mergedChildRegion = ri;
+          break;
+        }
+      }
+
+      assertTrue(mergedChildRegion != null);

Review comment:
       Use `assertNotNull` instead.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org