diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java index 3f1c575eef5..ec83553473e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java @@ -17,10 +17,7 @@ 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; @@ -28,51 +25,29 @@ * 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} 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} object to obtain locks from. - * @return a new {@code Striped}. - */ - @SuppressWarnings("unchecked") - public static Striped custom(int stripes, Supplier supplier) { - try { - Method custom = - Striped.class.getDeclaredMethod("custom", int.class, Supplier.class); - custom.setAccessible(true); - return (Striped) custom.invoke(null, stripes, supplier); - } catch (NoSuchMethodException | IllegalAccessException | - InvocationTargetException e) { - throw new IllegalStateException("Error creating custom Striped.", e); - } - } - /** * Creates a {@code Striped} 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} */ public static Striped readWriteLock(int stripes, boolean fair) { - return custom(stripes, () -> new ReentrantReadWriteLock(fair)); + return Striped.custom(stripes, () -> new ReentrantReadWriteLock(fair)); } } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/TestSimpleStriped.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/TestSimpleStriped.java index b7ab438eee1..ccd80b9fd24 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/TestSimpleStriped.java +++ b/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 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")); - } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java index 27aec9d00c9..698f005ca50 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java +++ b/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(); }