HDDS-12802. Avoid using reflection in SimpleStriped

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java

@@ -17,62 +17,37 @@
 
 package org.apache.hadoop.hdds.utils;
 
-import com.google.common.base.Supplier;
 import com.google.common.util.concurrent.Striped;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 /**
  * A custom factory to force creation of {@link Striped}
  * locks with fair order policy.
  *
- * The reason of this util is that today guava's {@link Striped} does not
- * support creating locks with fair policy, either supports a mechanism for
- * the client code to do that ({@link Striped#custom(int, Supplier)} is package
- * private). Ref: https://github.com/google/guava/issues/2514
+ * Guava's {@link Striped} does not natively support creating locks with
+ * fair ordering policy. Ref: https://github.com/google/guava/issues/2514
  *
- * So, we have to use reflection to forcibly support fair order policy with
- * {@link Striped}. When the above issue is resolved, we can remove this util.
+ * {@link Striped#custom(int, Supplier)} is now public, so we can call it
+ * directly. When fair policy is natively supported by {@link Striped}, this
+ * util can be removed.
  */
 public final class SimpleStriped {
 
   private SimpleStriped() {
   }
 
-
-  /**
-   * Creates a {@code Striped<L>} with eagerly initialized, strongly referenced
-   * locks. Every lock is obtained from the passed supplier.
-   *
-   * @param stripes the minimum number of stripes (locks) required.
-   * @param supplier a {@code Supplier<L>} object to obtain locks from.
-   * @return a new {@code Striped<L>}.
-   */
-  @SuppressWarnings("unchecked")
-  public static <L> Striped<L> custom(int stripes, Supplier<L> supplier) {
-    try {
-      Method custom =
-          Striped.class.getDeclaredMethod("custom", int.class, Supplier.class);
-      custom.setAccessible(true);
-      return (Striped<L>) custom.invoke(null, stripes, supplier);
-    } catch (NoSuchMethodException | IllegalAccessException |
-             InvocationTargetException e) {
-      throw new IllegalStateException("Error creating custom Striped.", e);
-    }
-  }
-
   /**
    * Creates a {@code Striped<ReadWriteLock>} with eagerly initialized,
    * strongly referenced read-write locks. Every lock is reentrant.
    *
    * @param stripes the minimum number of stripes (locks) required
+   * @param fair whether to use a fair ordering policy
    * @return a new {@code Striped<ReadWriteLock>}
    */
   public static Striped<ReadWriteLock> readWriteLock(int stripes,
       boolean fair) {
-    return custom(stripes, () -> new ReentrantReadWriteLock(fair));
+    return Striped.custom(stripes, () -> new ReentrantReadWriteLock(fair));
   }
 
 }

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/TestSimpleStriped.java

@@ -21,9 +21,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
 import com.google.common.util.concurrent.Striped;
-import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import org.junit.jupiter.api.Test;
 
@@ -51,16 +49,4 @@ private void testReadWriteLocks(boolean fair) {
     assertNotEquals(lock, striped.get("key2"));
   }
 
-  @Test
-  void testCustomStripes() {
-    int size = 128;
-    Striped<Lock> striped = SimpleStriped.custom(size,
-        ReentrantLock::new);
-    assertEquals(128, striped.size());
-    Lock lock = striped.get("key1");
-    // Ensure same key return same lock.
-    assertEquals(lock, striped.get("key1"));
-    // And different key (probably) return a different lock/
-    assertNotEquals(lock, striped.get("key2"));
-  }
 }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java

@@ -39,7 +39,6 @@
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
-import org.apache.hadoop.hdds.utils.SimpleStriped;
 import org.apache.hadoop.ozone.container.common.helpers.BlockData;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
 import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration;
@@ -66,7 +65,7 @@ public class ContainerChecksumTreeManager {
    * Creates one instance that should be used to coordinate all container checksum info within a datanode.
    */
   public ContainerChecksumTreeManager(ConfigurationSource conf) {
-    fileLocks = SimpleStriped.custom(conf.getObject(DatanodeConfiguration.class).getContainerChecksumLockStripes(),
+    fileLocks = Striped.custom(conf.getObject(DatanodeConfiguration.class).getContainerChecksumLockStripes(),
         () -> new ReentrantLock(true));
     metrics = ContainerMerkleTreeMetrics.create();
   }