From 9d7ee8af3cda2d22fa4eb735ff4f3c2023e38367 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Wed, 3 Mar 2021 18:20:46 +0200 Subject: [PATCH] Revert "APIBuilder: Remove RW-lock in JMX server repository wrapper" This reverts commit c2fc96be7160324e0fe887832413f15de18e0b0c. The RW-lock usage had a bug, which will be fixed in a follow up patch. --- .../com/scylladb/jmx/utils/APIBuilder.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/scylladb/jmx/utils/APIBuilder.java b/src/main/java/com/scylladb/jmx/utils/APIBuilder.java index 232d76a..5fe3f51 100644 --- a/src/main/java/com/scylladb/jmx/utils/APIBuilder.java +++ b/src/main/java/com/scylladb/jmx/utils/APIBuilder.java @@ -66,7 +66,7 @@ public class APIBuilder extends MBeanServerBuilder { private final Repository wrapped; - private final Object lock = new Object(); + private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final Map tableMBeans = new HashMap<>(); @@ -93,8 +93,11 @@ public class APIBuilder extends MBeanServerBuilder { if (!isTableMetricName(name)) { return wrapped.contains(name); } else { - synchronized(lock) { + lock.readLock().lock(); + try { return tableMBeans.containsKey(new TableMetricParams(name)); + } finally { + lock.readLock().unlock(); } } } @@ -113,20 +116,23 @@ public class APIBuilder extends MBeanServerBuilder { @Override public Integer getCount() { - synchronized(lock) { + lock.readLock().lock(); + try { return wrapped.getCount() + tableMBeans.size(); + } finally { + lock.readLock().unlock(); } } @Override public void addMBean(final DynamicMBean bean, final ObjectName name, final RegistrationContext ctx) throws InstanceAlreadyExistsException { - System.out.println(name); if (!isTableMetricName(name)) { wrapped.addMBean(bean, name, ctx); } else { final TableMetricParams key = new TableMetricParams(name); - synchronized(lock) { + lock.writeLock().lock(); + try { if (tableMBeans.containsKey(key)) { throw new InstanceAlreadyExistsException(name.toString()); } @@ -139,6 +145,8 @@ public class APIBuilder extends MBeanServerBuilder { } catch (RuntimeException x) { throw new RuntimeOperationsException(x); } + } finally { + lock.writeLock().unlock(); } } } @@ -149,7 +157,8 @@ public class APIBuilder extends MBeanServerBuilder { wrapped.remove(name, ctx); } else { final TableMetricParams key = new TableMetricParams(name); - synchronized(lock) { + lock.writeLock().lock(); + try { if (tableMBeans.remove(key) == null) { throw new InstanceNotFoundException(name.toString()); } @@ -162,6 +171,8 @@ public class APIBuilder extends MBeanServerBuilder { } catch (Exception x) { logger.log(SEVERE, "Unexpected error.", x); } + } finally { + lock.writeLock().lock(); } } } @@ -171,8 +182,11 @@ public class APIBuilder extends MBeanServerBuilder { if (!isTableMetricName(name)) { return wrapped.retrieve(name); } else { - synchronized(lock) { + lock.readLock().lock(); + try { return tableMBeans.get(new TableMetricParams(name)); + } finally { + lock.readLock().unlock(); } } } @@ -215,7 +229,8 @@ public class APIBuilder extends MBeanServerBuilder { name = pattern; } - synchronized(lock) { + lock.readLock().lock(); + try { // If pattern is not a pattern, retrieve this mbean ! if (!name.isPattern() && isTableMetricName(name)) { final DynamicMBean bean = tableMBeans.get(new TableMetricParams(name)); @@ -271,6 +286,8 @@ public class APIBuilder extends MBeanServerBuilder { addAllMatching(res, namePattern); } } + } finally { + lock.readLock().unlock(); } return res; }