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


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/balancer/TestCacheAwareLoadBalancerCostFunctions.java:
##########
@@ -251,12 +263,105 @@ public void testCacheCost() {
     }
   }
 
+  /**
+   * When block-cache persistence, cold regions (below
+   * {@link CacheAwareLoadBalancer#LOW_CACHE_RATIO_FOR_RELOCATION_KEY}) 
together with RS-reported
+   * block-cache free bytes inflate plausible best placement so weighted cache 
cost crosses
+   * {@code minCostNeedBalance}; {@link StochasticLoadBalancer#needsBalance} 
returns true even with
+   * evenly spread region-count skew.
+   */
+  @Test
+  public void testNeedsBalanceWhenLowCacheRatioRegionsAndFreeBlockCacheSpace() 
{
+    conf.set(HConstants.BUCKET_CACHE_PERSISTENT_PATH_KEY, 
"/tmp/prefetch.persistence");
+    CacheAwareLoadBalancer lb = newCacheAwareBalancer(conf);
+    int regionSizeMb = 64;
+    long cacheFreeInBytes = regionSizeMb * 1024L * 1024L;
+    // simulates a cache ratio lower than
+    // CacheAwareLoadBalancer.LOW_CACHE_RATIO_FOR_RELOCATION_DEFAULT
+    float simulatedCacheRatio = 0.1f;
+    Map<ServerName, List<RegionInfo>> clusterServers =
+      mockClusterServersUnsorted(new int[] { 1, 1 }, 1);
+    List<RegionInfo> regions = new ArrayList<>();
+    clusterServers.values().forEach(regions::addAll);
+    List<ServerName> serversList = getServersInInsertionOrder(clusterServers);
+    Map<ServerName, Long> blockCacheFree = new HashMap<>();
+    blockCacheFree.put(serversList.get(0), 0L);
+    blockCacheFree.put(serversList.get(1), cacheFreeInBytes);
+    BalancerClusterState cluster = new BalancerClusterState(clusterServers,
+      buildRegionLoads(regions, simulatedCacheRatio, regionSizeMb), null, null,
+      Collections.emptyMap(), blockCacheFree);
+    lb.initCosts(cluster);
+    assertTrue(lb.needsBalance(
+      
TableName.valueOf("testNeedsBalanceWhenLowCacheRatioRegionsAndFreeBlockCacheSpace"),
+      cluster));
+  }
+
+  /**
+   * Checks that needsBalance isn't true when regions report high cache ratios
+   */
+  @Test
+  public void testNeedsBalanceFalseWhenWarmRegionsDespiteFreeBlockCacheSpace() 
{
+    conf.set(HConstants.BUCKET_CACHE_PERSISTENT_PATH_KEY, 
"/tmp/prefetch.persistence");
+    CacheAwareLoadBalancer lb = newCacheAwareBalancer(conf);
+    int regionSizeMb = 64;
+    long cacheFreeInBytes = regionSizeMb * 1024L * 1024L;
+    Map<ServerName, List<RegionInfo>> clusterServers =
+      mockClusterServersUnsorted(new int[] { 1, 1 }, 1);
+    List<RegionInfo> all = new ArrayList<>();
+    clusterServers.values().forEach(all::addAll);
+    List<ServerName> serversList = getServersInInsertionOrder(clusterServers);
+    Map<ServerName, Long> blockCacheFree = new HashMap<>();
+    blockCacheFree.put(serversList.get(0), cacheFreeInBytes + 1024 * 1024);
+    blockCacheFree.put(serversList.get(1), cacheFreeInBytes + 1024 * 1024);
+    BalancerClusterState cluster =
+      new BalancerClusterState(clusterServers, buildRegionLoads(all, 1.0f, 
regionSizeMb), null,
+        null, Collections.emptyMap(), blockCacheFree);
+    lb.initCosts(cluster);
+    assertFalse(lb.needsBalance(
+      
TableName.valueOf("testNeedsBalanceFalseWhenWarmRegionsDespiteFreeBlockCacheSpace"),
+      cluster));
+  }
+
+  private static CacheAwareLoadBalancer newCacheAwareBalancer(Configuration 
cfg) {
+    CacheAwareLoadBalancer lb = new CacheAwareLoadBalancer();
+    lb.setClusterInfoProvider(new DummyClusterInfoProvider(cfg));
+    lb.loadConf(cfg);
+    return lb;
+  }
+
+  private static Map<String, Deque<BalancerRegionLoad>>
+    buildRegionLoads(Collection<RegionInfo> regions, float cachedRatio, int 
regionSizeMb) {
+    RegionMetrics rm = mock(RegionMetrics.class);
+    when(rm.getReadRequestCount()).thenReturn(0L);
+    when(rm.getCpRequestCount()).thenReturn(0L);
+    when(rm.getWriteRequestCount()).thenReturn(0L);
+    when(rm.getMemStoreSize()).thenReturn(Size.ZERO);
+    when(rm.getStoreFileSize()).thenReturn(Size.ZERO);
+    when(rm.getRegionSizeMB()).thenReturn(new Size(regionSizeMb, 
Size.Unit.MEGABYTE));
+    when(rm.getCurrentRegionCachedRatio()).thenReturn(cachedRatio);
+
+    BalancerRegionLoad brl = new BalancerRegionLoad(rm);
+    Map<String, Deque<BalancerRegionLoad>> loads = new HashMap<>();
+    for (RegionInfo ri : regions) {
+      ArrayDeque<BalancerRegionLoad> dq = new ArrayDeque<>(1);
+      dq.add(brl);
+      loads.put(ri.getRegionNameAsString(), dq);
+      loads.put(ri.getEncodedName(), dq);
+    }
+    return loads;
+  }
+
+  private static List<ServerName>
+    getServersInInsertionOrder(Map<ServerName, List<RegionInfo>> cluster) {
+    return new ArrayList<>(cluster.keySet());
+  }

Review Comment:
   Well, we have entire control over the map implementation instance passed 
from our tests, which is a LinkedHashMap, so no need to worry about this here.



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

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to