This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 00fe0fccf93 HDDS-12802. Avoid using reflection in SimpleStriped (#9814)
00fe0fccf93 is described below
commit 00fe0fccf9373b8101fee9d4a645a4e538406844
Author: Eric C. Ho <[email protected]>
AuthorDate: Sat Feb 28 03:43:03 2026 +0800
HDDS-12802. Avoid using reflection in SimpleStriped (#9814)
---
.../apache/hadoop/hdds/utils/SimpleStriped.java | 39 ++++------------------
.../hadoop/hdds/utils/TestSimpleStriped.java | 14 --------
.../checksum/ContainerChecksumTreeManager.java | 3 +-
3 files changed, 8 insertions(+), 48 deletions(-)
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<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));
}
}
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<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"));
- }
}
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();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
