[GitHub] [hbase] Apache9 commented on a change in pull request #295: HBASE-22551 TestMasterOperationsForRegionReplicas is flakey

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

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

 ##########
 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:
   Let me include this in the new patch, just a one line change.

----------------------------------------------------------------
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


With regards,
Apache Git Services