From ae1ec906801af4d47bfb57f30d9346ed67412d2b Mon Sep 17 00:00:00 2001
From: Nick Dimiduk <ndimiduk@apache.org>
Date: Thu, 12 Jan 2023 17:57:06 +0100
Subject: [PATCH] HBASE-27563 ChaosMonkey sometimes generates invalid
 boundaries for random item selection

Signed-off-by: Duo Zhang <zhangduo@apache.org>
---
 .../chaos/monkies/PolicyBasedChaosMonkey.java | 37 ++++++++++++++++---
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/hbase-it/src/test/java/org/apache/hadoop/hbase/chaos/monkies/PolicyBasedChaosMonkey.java b/hbase-it/src/test/java/org/apache/hadoop/hbase/chaos/monkies/PolicyBasedChaosMonkey.java
index ead5ccea51b2..28f87664f7f7 100644
--- a/hbase-it/src/test/java/org/apache/hadoop/hbase/chaos/monkies/PolicyBasedChaosMonkey.java
+++ b/hbase-it/src/test/java/org/apache/hadoop/hbase/chaos/monkies/PolicyBasedChaosMonkey.java
@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.chaos.monkies;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -30,6 +31,8 @@
 import org.apache.hadoop.hbase.IntegrationTestingUtility;
 import org.apache.hadoop.hbase.chaos.policies.Policy;
 import org.apache.hadoop.hbase.util.Pair;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
 
@@ -37,6 +40,7 @@
  * Chaos monkey that given multiple policies will run actions against the cluster.
  */
 public class PolicyBasedChaosMonkey extends ChaosMonkey {
+  private static final Logger LOG = LoggerFactory.getLogger(PolicyBasedChaosMonkey.class);
 
   private static final long ONE_SEC = 1000;
   private static final long ONE_MIN = 60 * ONE_SEC;
@@ -116,13 +120,31 @@ public static <T> T selectWeightedRandomItem(List<Pair<T, Integer>> items) {
 
   /** Selects and returns ceil(ratio * items.length) random items from the given array */
   public static <T> List<T> selectRandomItems(T[] items, float ratio) {
-    int selectedNumber = (int) Math.ceil(items.length * ratio);
+    /*
+     * N.b. `ratio` values are not validated. Be aware of excessive values and floating point
+     * arithmetic rounding. Guard against negative input to Random#next() and exceeding boundaries
+     * in call to List#subList.
+     */
 
-    List<T> originalItems = Arrays.asList(items);
-    Collections.shuffle(originalItems);
+    // clamp ratio to [0.0,1.0]
+    ratio = Math.max(Math.min(ratio, 1.0f), 0.0f);
 
-    int startIndex = ThreadLocalRandom.current().nextInt(items.length - selectedNumber);
-    return originalItems.subList(startIndex, startIndex + selectedNumber);
+    final int selectedNumber = (int) Math.ceil(items.length * ratio);
+
+    // shuffle a copy of the input, not the input.
+    final List<T> shuffledItems = new ArrayList<>(items.length);
+    shuffledItems.addAll(Arrays.asList(items));
+    Collections.shuffle(shuffledItems);
+
+    if (selectedNumber >= items.length) {
+      return shuffledItems;
+    }
+
+    // apply basic sanity check on sublist selection range.
+    final int startIndex =
+      Math.max(0, ThreadLocalRandom.current().nextInt(items.length - selectedNumber));
+    final int endIndex = Math.min(items.length, startIndex + selectedNumber);
+    return shuffledItems.subList(startIndex, endIndex);
   }
 
   @Override
@@ -151,7 +173,10 @@ public boolean isStopped() {
 
   @Override
   public void waitForStop() throws InterruptedException {
-    monkeyThreadPool.awaitTermination(1, TimeUnit.MINUTES);
+    if (!monkeyThreadPool.awaitTermination(1, TimeUnit.MINUTES)) {
+      LOG.warn("Some pool threads failed to terminate. Forcing. {}", monkeyThreadPool);
+      monkeyThreadPool.shutdownNow();
+    }
   }
 
   @Override