From 8c7d4ff49c3308ebe67748d786ddf5afb037804e Mon Sep 17 00:00:00 2001 From: echonesis Date: Tue, 24 Feb 2026 15:57:44 +0800 Subject: [PATCH 1/3] HDDS-12802. Avoid using reflection in SimpleStriped --- .../util/concurrent}/SimpleStriped.java | 23 +++++-------------- .../common/util/concurrent/package-info.java | 21 +++++++++++++++++ .../hadoop/hdds/utils/TestSimpleStriped.java | 1 + .../ContainerChecksumTreeManager.java | 2 +- .../ozone/om/lock/OzoneManagerLock.java | 2 +- 5 files changed, 30 insertions(+), 19 deletions(-) rename hadoop-hdds/common/src/main/java/{org/apache/hadoop/hdds/utils => com/google/common/util/concurrent}/SimpleStriped.java (74%) create mode 100644 hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java b/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/SimpleStriped.java similarity index 74% rename from hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java rename to hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/SimpleStriped.java index 3f1c575eef5..564d1992352 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java +++ b/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/SimpleStriped.java @@ -15,12 +15,9 @@ * limitations under the License. */ -package org.apache.hadoop.hdds.utils; +package com.google.common.util.concurrent; 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; @@ -33,15 +30,15 @@ * the client code to do that ({@link Striped#custom(int, Supplier)} is package * private). 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. + * So, we place this class in the same package as {@link Striped} to directly + * call the package-private method without reflection. + * When the above issue is resolved, we can remove this util. */ public final class SimpleStriped { private SimpleStriped() { } - /** * Creates a {@code Striped} with eagerly initialized, strongly referenced * locks. Every lock is obtained from the passed supplier. @@ -50,17 +47,8 @@ private SimpleStriped() { * @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); - } + return Striped.custom(stripes, supplier); } /** @@ -68,6 +56,7 @@ public static Striped custom(int stripes, Supplier supplier) { * 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, diff --git a/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java b/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java new file mode 100644 index 00000000000..377a0a1e139 --- /dev/null +++ b/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java @@ -0,0 +1,21 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Classes using the Guava internal APIs. + */ +package com.google.common.util.concurrent; 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..ee5ef982413 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 @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import com.google.common.util.concurrent.SimpleStriped; import com.google.common.util.concurrent.Striped; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; 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..9c1028052f5 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 @@ -23,6 +23,7 @@ import static org.apache.hadoop.ozone.util.MetricUtil.captureLatencyNs; import com.google.common.annotations.VisibleForTesting; +import com.google.common.util.concurrent.SimpleStriped; import com.google.common.util.concurrent.Striped; import java.io.File; import java.io.FileNotFoundException; @@ -39,7 +40,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; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java index f567f17766b..580f79b5d91 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java @@ -25,6 +25,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.SimpleStriped; import com.google.common.util.concurrent.Striped; import java.util.ArrayList; import java.util.Arrays; @@ -44,7 +45,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.utils.CompositeKey; -import org.apache.hadoop.hdds.utils.SimpleStriped; import org.apache.hadoop.ipc_.ProcessingDetails.Timing; import org.apache.hadoop.ipc_.Server; import org.apache.hadoop.util.Time; From 0bbb848528431c2ffcc03f719c241c2911a4c003 Mon Sep 17 00:00:00 2001 From: echonesis Date: Wed, 25 Feb 2026 10:11:27 +0800 Subject: [PATCH 2/3] fix: move SimpleStriped out of Guava package to hdds.utils --- .../common/util/concurrent/package-info.java | 21 ------------------- .../hadoop/hdds/utils}/SimpleStriped.java | 15 +++++++------ .../hadoop/hdds/utils/TestSimpleStriped.java | 1 - .../ContainerChecksumTreeManager.java | 2 +- .../ozone/om/lock/OzoneManagerLock.java | 2 +- 5 files changed, 9 insertions(+), 32 deletions(-) delete mode 100644 hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java rename hadoop-hdds/common/src/main/java/{com/google/common/util/concurrent => org/apache/hadoop/hdds/utils}/SimpleStriped.java (79%) diff --git a/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java b/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java deleted file mode 100644 index 377a0a1e139..00000000000 --- a/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/package-info.java +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Classes using the Guava internal APIs. - */ -package com.google.common.util.concurrent; diff --git a/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/SimpleStriped.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java similarity index 79% rename from hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/SimpleStriped.java rename to hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java index 564d1992352..dffd2d5d7f5 100644 --- a/hadoop-hdds/common/src/main/java/com/google/common/util/concurrent/SimpleStriped.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java @@ -15,9 +15,10 @@ * limitations under the License. */ -package com.google.common.util.concurrent; +package org.apache.hadoop.hdds.utils; import com.google.common.base.Supplier; +import com.google.common.util.concurrent.Striped; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -25,14 +26,12 @@ * 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 place this class in the same package as {@link Striped} to directly - * call the package-private method without reflection. - * 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 { 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 ee5ef982413..b7ab438eee1 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 @@ -20,7 +20,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; -import com.google.common.util.concurrent.SimpleStriped; import com.google.common.util.concurrent.Striped; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; 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 9c1028052f5..27aec9d00c9 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 @@ -23,7 +23,6 @@ import static org.apache.hadoop.ozone.util.MetricUtil.captureLatencyNs; import com.google.common.annotations.VisibleForTesting; -import com.google.common.util.concurrent.SimpleStriped; import com.google.common.util.concurrent.Striped; import java.io.File; import java.io.FileNotFoundException; @@ -40,6 +39,7 @@ 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; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java index 580f79b5d91..f567f17766b 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java @@ -25,7 +25,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; -import com.google.common.util.concurrent.SimpleStriped; import com.google.common.util.concurrent.Striped; import java.util.ArrayList; import java.util.Arrays; @@ -45,6 +44,7 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.utils.CompositeKey; +import org.apache.hadoop.hdds.utils.SimpleStriped; import org.apache.hadoop.ipc_.ProcessingDetails.Timing; import org.apache.hadoop.ipc_.Server; import org.apache.hadoop.util.Time; From 6abcb910d64ba91c466b299704f15ffc47099692 Mon Sep 17 00:00:00 2001 From: echonesis Date: Sat, 28 Feb 2026 01:03:17 +0800 Subject: [PATCH 3/3] fix: remove SimpleStriped.custom and use Striped.custom directly --- .../apache/hadoop/hdds/utils/SimpleStriped.java | 15 +-------------- .../hadoop/hdds/utils/TestSimpleStriped.java | 14 -------------- .../checksum/ContainerChecksumTreeManager.java | 3 +-- 3 files changed, 2 insertions(+), 30 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 dffd2d5d7f5..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,7 +17,6 @@ package org.apache.hadoop.hdds.utils; -import com.google.common.base.Supplier; import com.google.common.util.concurrent.Striped; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -38,18 +37,6 @@ 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}. - */ - public static Striped custom(int stripes, Supplier supplier) { - return Striped.custom(stripes, supplier); - } - /** * Creates a {@code Striped} with eagerly initialized, * strongly referenced read-write locks. Every lock is reentrant. @@ -60,7 +47,7 @@ public static Striped custom(int stripes, Supplier supplier) { */ 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(); }