wchevreuil commented on code in PR #5376:
URL: https://github.com/apache/hbase/pull/5376#discussion_r1309943247


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtil.java:
##########
@@ -3642,6 +3662,110 @@ public boolean evaluate() throws IOException {
     };
   }
 
+  /**
+   * Returns a {@Link Predicate} for checking that all the regions for a table 
are prefetched
+   */
+  public Waiter.Predicate<IOException>
+    predicateAllRegionsForTableArePrefetched(final TableName tableName) {
+    return new ExplainingPredicate<IOException>() {
+      @Override
+      public String explainFailure() throws IOException {
+        return "Not all the regions for the table " + 
tableName.getNameAsString()
+          + " are prefetched";
+      }
+
+      @Override
+      public boolean evaluate() throws IOException {
+        List<HRegion> regions = getMiniHBaseCluster().getRegions(tableName);
+        int totalRegionCount = regions.size();
+        AtomicInteger prefetchedRegionCount = new AtomicInteger();
+        for (HRegion r : regions) {
+          
getMiniHBaseCluster().getClusterMetrics().getLiveServerMetrics().forEach((sn, 
sm) -> {
+            sm.getRegionMetrics().forEach((rn, rm) -> {
+              String regionNameAsString = 
r.getRegionInfo().getRegionNameAsString();
+              String regionString = rm.getNameAsString();
+              if (regionNameAsString.equals(regionString)) {
+                if (rm.getCurrentRegionCachedRatio() == 1.0f) {
+                  prefetchedRegionCount.getAndIncrement();
+                }
+              }
+            });
+          });
+        }
+        return getAdmin().tableExists(tableName) && totalRegionCount == 
prefetchedRegionCount.get();
+      }
+    };
+  }
+
+  /**
+   * Returns a {@Link Predicate} for checking that at least one region for the 
table is prefetched
+   */
+  public Waiter.Predicate<IOException> 
predicateAtLeastOneRegionIsPrefetchedOnServer(
+    final TableName tableName, final ServerName serverName) {
+    return new ExplainingPredicate<IOException>() {
+      @Override
+      public String explainFailure() throws IOException {
+        return "No Regions for table " + tableName.getNameAsString() + " 
prefetched on server "
+          + serverName.getAddress();
+      }
+
+      @Override
+      public boolean evaluate() throws IOException {
+        List<HRegion> regions = getMiniHBaseCluster().getRegions(tableName);
+        AtomicInteger prefetchedRegionCount = new AtomicInteger();
+        ServerMetrics sm =
+          
getMiniHBaseCluster().getClusterMetrics().getLiveServerMetrics().get(serverName);
+        for (HRegion r : regions) {
+          sm.getRegionMetrics().forEach((rn, rm) -> {
+            if (
+              
r.getRegionInfo().getRegionNameAsString().equals(rm.getNameAsString())
+                && rm.getCurrentRegionCachedRatio() == 1.0f
+            ) {
+              prefetchedRegionCount.getAndIncrement();
+            }
+          });
+        }
+        return getAdmin().tableExists(tableName) && 
prefetchedRegionCount.get() > 0;
+      }
+    };
+  }
+
+  /**
+   * Returns a {@Link Predicate} for checking that more than half of the 
regions for the table are
+   * prefetched
+   */
+  public Waiter.Predicate<IOException>
+    predicateMajorityRegionsArePrefetched(final TableName tableName) {
+    return new ExplainingPredicate<IOException>() {
+      @Override
+      public String explainFailure() throws IOException {
+        return "No Regions for table " + tableName.getNameAsString() + " 
prefetched";
+      }
+
+      @Override
+      public boolean evaluate() throws IOException {
+        List<HRegion> regions = getMiniHBaseCluster().getRegions(tableName);
+        int totalRegionCount = regions.size();
+        AtomicInteger prefetchedRegionCount = new AtomicInteger();
+        for (HRegion r : regions) {
+          
getMiniHBaseCluster().getClusterMetrics().getLiveServerMetrics().forEach((sn, 
sm) -> {
+            sm.getRegionMetrics().forEach((rn, rm) -> {
+              String regionNameAsString = 
r.getRegionInfo().getRegionNameAsString();
+              String regionString = rm.getNameAsString();
+              if (regionNameAsString.equals(regionString)) {
+                if (rm.getCurrentRegionCachedRatio() == 1.0f) {
+                  prefetchedRegionCount.getAndIncrement();
+                }
+              }
+            });
+          });
+        }
+        return getAdmin().tableExists(tableName)
+          && (float) prefetchedRegionCount.get() / totalRegionCount > 0.5f;
+      }
+    };
+  }
+
   /**

Review Comment:
   These utility methods are all specific for this new balance function, so it 
should not be here in HBaseTestingUtil. Please move it to your tests classes. 
Maybe make an abstract parent class which all your tests would extend?



##########
hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtil.java:
##########
@@ -2906,6 +2907,25 @@ public boolean ensureSomeRegionServersAvailable(final 
int num) throws IOExceptio
     return startedServer;
   }
 
+  /**
+   * Waits for all the regions of a table to be prefetched fully
+   * @param table Table to be wait on.
+   */

Review Comment:
   The javadoc seems out of sync with the method behaviour.



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

To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org

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

Reply via email to