This is an automated email from the ASF dual-hosted git repository.
ritesh 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 af72296b0a HDDS-9237. Enable lock order fairness for striped locks.
(#5256)
af72296b0a is described below
commit af72296b0a2722b69e104aae7c5f8dcdab438cc0
Author: Duong Nguyen <[email protected]>
AuthorDate: Tue Sep 12 13:50:47 2023 -0700
HDDS-9237. Enable lock order fairness for striped locks. (#5256)
---
.../apache/hadoop/hdds/utils/SimpleStriped.java | 81 ++++++++++++++++++++++
.../hadoop/hdds/utils/TestSimpleStriped.java | 68 ++++++++++++++++++
.../hadoop/ozone/om/lock/OzoneManagerLock.java | 27 ++++----
3 files changed, 161 insertions(+), 15 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
new file mode 100644
index 0000000000..a3840b995d
--- /dev/null
+++
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/SimpleStriped.java
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ *
+ */
+
+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
+ *
+ * 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.
+ */
+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
+ * @return a new {@code Striped<ReadWriteLock>}
+ */
+ public static Striped<ReadWriteLock> readWriteLock(int stripes,
+ boolean fair) {
+ return 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
new file mode 100644
index 0000000000..ff9ede44af
--- /dev/null
+++
b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/TestSimpleStriped.java
@@ -0,0 +1,68 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.
+ *
+ */
+package org.apache.hadoop.hdds.utils;
+
+import com.google.common.util.concurrent.Striped;
+import org.junit.jupiter.api.Test;
+
+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 static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+/**
+ * Test cases for {@link SimpleStriped}.
+ */
+public class TestSimpleStriped {
+ @Test
+ void testReadWriteLocks() {
+ testReadWriteLocks(true);
+ testReadWriteLocks(false);
+ }
+
+ private void testReadWriteLocks(boolean fair) {
+ Striped<ReadWriteLock> striped = SimpleStriped.readWriteLock(128,
+ fair);
+ assertEquals(128, striped.size());
+ ReadWriteLock lock = striped.get("key1");
+ assertEquals(fair, ((ReentrantReadWriteLock) lock).isFair());
+
+ // Ensure same key return same lock.
+ assertEquals(lock, striped.get("key1"));
+
+ // And different key (probably) return a different lock/
+ 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-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 39d74cb1ee..975a982842 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
@@ -31,12 +31,15 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.Striped;
+import org.apache.hadoop.hdds.utils.SimpleStriped;
import org.apache.hadoop.util.Time;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_FAIR_LOCK;
+import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_FAIR_LOCK_DEFAULT;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_STRIPED_LOCK_SIZE_DEFAULT;
import static
org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_STRIPED_LOCK_SIZE_PREFIX;
import static org.apache.hadoop.hdds.utils.CompositeKey.combineKeys;
@@ -99,14 +102,6 @@ public class OzoneManagerLock implements IOzoneManagerLock {
*/
public OzoneManagerLock(ConfigurationSource conf) {
omLockMetrics = OMLockMetrics.create();
-
- // TODO: for now, guava Striped doesn't allow create a striped
- // ReadWriteLock with fair-lock yet.
- // https://github.com/google/guava/issues/2514.
- // We may have to consider implement our own striped locks.
- //boolean fair = conf.getBoolean(OZONE_MANAGER_FAIR_LOCK,
- // OZONE_MANAGER_FAIR_LOCK_DEFAULT);
-
Map<Resource, Striped<ReadWriteLock>> stripedLockMap =
new EnumMap<>(Resource.class);
for (Resource r : Resource.values()) {
@@ -117,17 +112,19 @@ public class OzoneManagerLock implements
IOzoneManagerLock {
private Striped<ReadWriteLock> createStripeLock(Resource r,
ConfigurationSource conf) {
+ boolean fair = conf.getBoolean(OZONE_MANAGER_FAIR_LOCK,
+ OZONE_MANAGER_FAIR_LOCK_DEFAULT);
String stripeSizeKey = OZONE_MANAGER_STRIPED_LOCK_SIZE_PREFIX +
r.getName().toLowerCase();
int size = conf.getInt(stripeSizeKey,
OZONE_MANAGER_STRIPED_LOCK_SIZE_DEFAULT);
- return Striped.readWriteLock(size);
+ return SimpleStriped.readWriteLock(size, fair);
}
private ReentrantReadWriteLock getLock(Resource resource, String... keys) {
- Striped<ReadWriteLock> lockStriped = stripedLockByResource.get(resource);
+ Striped<ReadWriteLock> striped = stripedLockByResource.get(resource);
Object key = combineKeys(keys);
- return (ReentrantReadWriteLock) lockStriped.get(key);
+ return (ReentrantReadWriteLock) striped.get(key);
}
/**
@@ -267,12 +264,12 @@ public class OzoneManagerLock implements
IOzoneManagerLock {
LOG.error(errorMessage);
throw new RuntimeException(errorMessage);
} else {
- Striped<ReadWriteLock> stripedLocks =
+ Striped<ReadWriteLock> striped =
stripedLockByResource.get(Resource.USER_LOCK);
// The result of bulkGet is always sorted in a consistent order.
// This prevents deadlocks.
Iterable<ReadWriteLock> locks =
- stripedLocks.bulkGet(Arrays.asList(firstUser, secondUser));
+ striped.bulkGet(Arrays.asList(firstUser, secondUser));
for (ReadWriteLock lock : locks) {
lock.writeLock().lock();
}
@@ -290,10 +287,10 @@ public class OzoneManagerLock implements
IOzoneManagerLock {
*/
@Override
public void releaseMultiUserLock(String firstUser, String secondUser) {
- Striped<ReadWriteLock> stripedLocks =
+ Striped<ReadWriteLock> striped =
stripedLockByResource.get(Resource.USER_LOCK);
Iterable<ReadWriteLock> locks =
- stripedLocks.bulkGet(Arrays.asList(firstUser, secondUser));
+ striped.bulkGet(Arrays.asList(firstUser, secondUser));
for (ReadWriteLock lock : locks) {
lock.writeLock().unlock();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
