Apache9 commented on a change in pull request #295: HBASE-22551 
TestMasterOperationsForRegionReplicas is flakey
URL: https://github.com/apache/hbase/pull/295#discussion_r291835737
 
 

 ##########
 File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java
 ##########
 @@ -192,86 +202,95 @@ public void testCreateTableWithMultipleReplicas() throws 
Exception {
       // figure current cluster ports and pass them in on next cluster start 
so new cluster comes
       // up at same coordinates -- and the assignment retention logic has a 
chance to cut in.
       List<Integer> rsports = new ArrayList<>();
-      for (JVMClusterUtil.RegionServerThread rst:
-          TEST_UTIL.getHBaseCluster().getLiveRegionServerThreads()) {
+      for (JVMClusterUtil.RegionServerThread rst : TEST_UTIL.getHBaseCluster()
+        .getLiveRegionServerThreads()) {
         
rsports.add(rst.getRegionServer().getRpcServer().getListenerAddress().getPort());
       }
       TEST_UTIL.shutdownMiniHBaseCluster();
-      StartMiniClusterOption option = StartMiniClusterOption.builder()
-          .numRegionServers(numSlaves).rsPorts(rsports).build();
+      StartMiniClusterOption option =
+        
StartMiniClusterOption.builder().numRegionServers(numSlaves).rsPorts(rsports).build();
       TEST_UTIL.startMiniHBaseCluster(option);
-      TEST_UTIL.waitTableAvailable(tableName);
+      TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
+      TEST_UTIL.waitUntilNoRegionsInTransition();
       validateFromSnapshotFromMeta(TEST_UTIL, tableName, numRegions, 
numReplica,
         ADMIN.getConnection());
 
       // Now shut the whole cluster down, and verify regions are assigned even 
if there is only
       // one server running
       TEST_UTIL.shutdownMiniHBaseCluster();
       TEST_UTIL.startMiniHBaseCluster();
-      TEST_UTIL.waitTableAvailable(tableName);
+      TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
+      TEST_UTIL.waitUntilNoRegionsInTransition();
       validateSingleRegionServerAssignment(ADMIN.getConnection(), numRegions, 
numReplica);
-      for (int i = 1; i < numSlaves; i++) { //restore the cluster
+      for (int i = 1; i < numSlaves; i++) { // restore the cluster
         TEST_UTIL.getMiniHBaseCluster().startRegionServer();
       }
 
       // Check on alter table
       ADMIN.disableTable(tableName);
-      assert(ADMIN.isTableDisabled(tableName));
-      //increase the replica
-      desc.setRegionReplication(numReplica + 1);
-      ADMIN.modifyTable(desc);
+      assertTrue(ADMIN.isTableDisabled(tableName));
+      // increase the replica
+      ADMIN.modifyTable(
+        
TableDescriptorBuilder.newBuilder(desc).setRegionReplication(numReplica + 
1).build());
       ADMIN.enableTable(tableName);
       LOG.info(ADMIN.getDescriptor(tableName).toString());
-      assert(ADMIN.isTableEnabled(tableName));
-      List<RegionInfo> regions = TEST_UTIL.getMiniHBaseCluster().getMaster().
-          
getAssignmentManager().getRegionStates().getRegionsOfTable(tableName);
-      assertTrue("regions.size=" + regions.size() + ", numRegions=" + 
numRegions +
-          ", numReplica=" + numReplica, regions.size() == numRegions * 
(numReplica + 1));
+      assertTrue(ADMIN.isTableEnabled(tableName));
+      TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
+      TEST_UTIL.waitUntilNoRegionsInTransition();
+      List<RegionInfo> regions = 
TEST_UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager()
+        .getRegionStates().getRegionsOfTable(tableName);
+      assertTrue("regions.size=" + regions.size() + ", numRegions=" + 
numRegions + ", numReplica=" +
+        numReplica, regions.size() == numRegions * (numReplica + 1));
 
-      //decrease the replica(earlier, table was modified to have a replica 
count of numReplica + 1)
+      // decrease the replica(earlier, table was modified to have a replica 
count of numReplica + 1)
       ADMIN.disableTable(tableName);
-      desc.setRegionReplication(numReplica);
-      ADMIN.modifyTable(desc);
+      ADMIN.modifyTable(
+        
TableDescriptorBuilder.newBuilder(desc).setRegionReplication(numReplica).build());
       ADMIN.enableTable(tableName);
-      assert(ADMIN.isTableEnabled(tableName));
-      regions = TEST_UTIL.getMiniHBaseCluster().getMaster()
-          
.getAssignmentManager().getRegionStates().getRegionsOfTable(tableName);
-      assert(regions.size() == numRegions * numReplica);
-      //also make sure the meta table has the replica locations removed
+      assertTrue(ADMIN.isTableEnabled(tableName));
+      TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
+      TEST_UTIL.waitUntilNoRegionsInTransition();
+      regions = 
TEST_UTIL.getMiniHBaseCluster().getMaster().getAssignmentManager().getRegionStates()
+        .getRegionsOfTable(tableName);
+      assertEquals(numRegions * numReplica, regions.size());
+      // also make sure the meta table has the replica locations removed
       hris = MetaTableAccessor.getTableRegions(ADMIN.getConnection(), 
tableName);
-      assert(hris.size() == numRegions * numReplica);
-      //just check that the number of default replica regions in the meta 
table are the same
-      //as the number of regions the table was created with, and the count of 
the
-      //replicas is numReplica for each region
+      assertEquals(numRegions * numReplica, hris.size());
+      // just check that the number of default replica regions in the meta 
table are the same
+      // as the number of regions the table was created with, and the count of 
the
+      // replicas is numReplica for each region
       Map<RegionInfo, Integer> defaultReplicas = new HashMap<>();
       for (RegionInfo hri : hris) {
         Integer i;
         RegionInfo regionReplica0 = 
RegionReplicaUtil.getRegionInfoForDefaultReplica(hri);
         defaultReplicas.put(regionReplica0,
-            (i = defaultReplicas.get(regionReplica0)) == null ? 1 : i + 1);
+          (i = defaultReplicas.get(regionReplica0)) == null ? 1 : i + 1);
       }
-      assert(defaultReplicas.size() == numRegions);
+      assertEquals(numRegions, defaultReplicas.size());
       Collection<Integer> counts = new HashSet<>(defaultReplicas.values());
-      assert(counts.size() == 1 && counts.contains(numReplica));
+      assertEquals(1, counts.size());
+      assertTrue(counts.contains(numReplica));
     } finally {
       ADMIN.disableTable(tableName);
       ADMIN.deleteTable(tableName);
     }
   }
 
-  @Test @Ignore("Enable when we have support for alter_table- HBASE-10361")
+  @Test
+  @Ignore("Enable when we have support for alter_table- HBASE-10361")
 
 Review comment:
   Oh the test fails locally for me... Then let's use a new issue to address 
it...

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to