You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ch...@apache.org on 2016/06/22 11:18:00 UTC
svn commit: r1749654 - in /jackrabbit/oak/branches/1.4: ./
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProvider.java
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProviderTest.java
Author: chetanm
Date: Wed Jun 22 11:18:00 2016
New Revision: 1749654
URL: http://svn.apache.org/viewvc?rev=1749654&view=rev
Log:
OAK-4483 - Remove synchronized access requirement from MetricStatisticsProvider#getStats
Merge 1749275
Modified:
jackrabbit/oak/branches/1.4/ (props changed)
jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProvider.java
jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProviderTest.java
Propchange: jackrabbit/oak/branches/1.4/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Jun 22 11:18:00 2016
@@ -1,3 +1,3 @@
/jackrabbit/oak/branches/1.0:1665962
-/jackrabbit/oak/trunk:1733615,1733875,1733913,1733929,1734230,1734254,1734279,1734941,1735052,1735405,1735484,1735549,1735564,1735588,1735622,1735638,1735919,1735983,1736176,1737309-1737310,1737334,1737349,1737998,1738004,1738775,1738795,1738833,1738950,1738957,1738963,1739894,1740116,1740625-1740626,1740971,1741032,1741339,1741343,1742520,1742888,1742916,1743097,1743172,1743343,1744265,1744959,1745038,1745197,1745368,1746086,1746117,1746342,1746345,1746696,1746981,1747492,1748505,1748553,1748870,1749350,1749464,1749475
+/jackrabbit/oak/trunk:1733615,1733875,1733913,1733929,1734230,1734254,1734279,1734941,1735052,1735405,1735484,1735549,1735564,1735588,1735622,1735638,1735919,1735983,1736176,1737309-1737310,1737334,1737349,1737998,1738004,1738775,1738795,1738833,1738950,1738957,1738963,1739894,1740116,1740625-1740626,1740971,1741032,1741339,1741343,1742520,1742888,1742916,1743097,1743172,1743343,1744265,1744959,1745038,1745197,1745368,1746086,1746117,1746342,1746345,1746696,1746981,1747492,1748505,1748553,1748870,1749275,1749350,1749464,1749475
/jackrabbit/trunk:1345480
Modified: jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProvider.java?rev=1749654&r1=1749653&r2=1749654&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProvider.java (original)
+++ jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProvider.java Wed Jun 22 11:18:00 2016
@@ -21,7 +21,7 @@ package org.apache.jackrabbit.oak.plugin
import java.io.Closeable;
import java.util.Hashtable;
-import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
@@ -59,7 +59,7 @@ public class MetricStatisticsProvider im
private static final String JMX_TYPE_METRICS = "Metrics";
- private final Map<String, Stats> statsRegistry = Maps.newHashMap();
+ private final ConcurrentMap<String, Stats> statsRegistry = Maps.newConcurrentMap();
private final MetricRegistry registry;
private final JmxReporter reporter;
private final RepositoryStatisticsImpl repoStats;
@@ -125,17 +125,25 @@ public class MetricStatisticsProvider im
return repoStats;
}
- private synchronized <T extends Stats> T getStats(String name, StatsBuilder<T> builder, StatsOptions options) {
+ private <T extends Stats> T getStats(String name, StatsBuilder<T> builder, StatsOptions options) {
Stats stats = statsRegistry.get(name);
+ //Use double locking pattern. The map should get populated with required set
+ //during startup phase so for later calls should not hit the synchronized block
if (stats == null) {
- if (options.isOnlyMetricEnabled()) {
- stats = builder.newMetric(this, name);
- } else if (options.isOnlyTimeSeriesEnabled()){
- stats = new SimpleStats(getTimerSeriesStats(name, builder), builder.getType());
- } else {
- stats = builder.newComposite(getTimerSeriesStats(name, builder), this, name);
+ synchronized (this){
+ stats = statsRegistry.get(name);
+ //If still null then go ahead and create it within lock
+ if (stats == null){
+ if (options.isOnlyMetricEnabled()) {
+ stats = builder.newMetric(this, name);
+ } else if (options.isOnlyTimeSeriesEnabled()) {
+ stats = new SimpleStats(getTimerSeriesStats(name, builder), builder.getType());
+ } else {
+ stats = builder.newComposite(getTimerSeriesStats(name, builder), this, name);
+ }
+ statsRegistry.put(name, stats);
+ }
}
- statsRegistry.put(name, stats);
}
if (builder.isInstance(stats)) {
Modified: jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProviderTest.java?rev=1749654&r1=1749653&r2=1749654&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProviderTest.java (original)
+++ jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/metric/MetricStatisticsProviderTest.java Wed Jun 22 11:18:00 2016
@@ -20,7 +20,11 @@
package org.apache.jackrabbit.oak.plugins.metric;
import java.lang.management.ManagementFactory;
+import java.util.List;
+import java.util.Queue;
import java.util.Set;
+import java.util.concurrent.ConcurrentLinkedDeque;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
@@ -32,6 +36,9 @@ import javax.management.Query;
import javax.management.QueryExp;
import com.codahale.metrics.JmxReporter;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.google.common.util.concurrent.Uninterruptibles;
import org.apache.jackrabbit.api.stats.RepositoryStatistics.Type;
import org.apache.jackrabbit.oak.stats.CounterStats;
import org.apache.jackrabbit.oak.stats.HistogramStats;
@@ -142,6 +149,44 @@ public class MetricStatisticsProviderTes
assertInstanceOf(statsProvider.getTimer("fooTimer", StatsOptions.DEFAULT), CompositeStats.class);
}
+ @Test
+ public void concurrentAccess() throws Exception{
+ //Queue is used to collect instances with minimal overhead in concurrent scenario
+ final Queue<MeterStats> statsQueue = new ConcurrentLinkedDeque<MeterStats>();
+ List<Thread> threads = Lists.newArrayList();
+ int numWorker = 5;
+ final CountDownLatch latch = new CountDownLatch(1);
+ for (int i = 0; i < numWorker; i++) {
+ threads.add(new Thread(new Runnable() {
+ @Override
+ public void run() {
+ Uninterruptibles.awaitUninterruptibly(latch);
+ statsQueue.add(statsProvider.getMeter("foo", StatsOptions.DEFAULT));
+ }
+ }));
+ }
+
+ for (Thread t : threads){
+ t.start();
+ }
+
+ latch.countDown();
+
+ for (Thread t : threads){
+ t.join();
+ }
+
+ //Assert that we get same reference for every call
+ Set<MeterStats> statsSet = Sets.newIdentityHashSet();
+
+ for (MeterStats m : statsQueue){
+ statsSet.add(m);
+ }
+
+ assertEquals(1, statsSet.size());
+
+ }
+
@After
public void cleanup() {
statsProvider.close();