From 3bb13c03db9366724be756b65004f111aead1c20 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Wed, 23 Sep 2015 23:06:09 +0900 Subject: [PATCH] More refactors to isolate metrics from internal pool classes --- .../zaxxer/hikari/metrics/MetricsTracker.java | 22 +----- .../dropwizard/CodaHaleMetricsTracker.java | 26 ++----- .../com/zaxxer/hikari/pool/HikariPool.java | 6 +- .../java/com/zaxxer/hikari/pool/PoolBase.java | 21 ++---- .../com/zaxxer/hikari/util/ClockSource.java | 71 ++++++++++++++++++- .../com/zaxxer/hikari/util/ConcurrentBag.java | 20 +++--- 6 files changed, 94 insertions(+), 72 deletions(-) diff --git a/src/main/java/com/zaxxer/hikari/metrics/MetricsTracker.java b/src/main/java/com/zaxxer/hikari/metrics/MetricsTracker.java index bbb9823d..fcbf549c 100644 --- a/src/main/java/com/zaxxer/hikari/metrics/MetricsTracker.java +++ b/src/main/java/com/zaxxer/hikari/metrics/MetricsTracker.java @@ -23,18 +23,15 @@ package com.zaxxer.hikari.metrics; */ public class MetricsTracker implements AutoCloseable { - public static final MetricsTimerContext NO_CONTEXT = new MetricsTimerContext(); - public MetricsTracker() { } - public MetricsTimerContext recordConnectionRequest() + public void recordConnectionAcquireNanos(final long elapsedAcquireNano) { - return NO_CONTEXT; } - public void recordConnectionUsage(final long elapsedLastBorrowed) + public void recordConnectionUsageMillis(final long elapsedLastBorrowed) { } @@ -42,19 +39,4 @@ public class MetricsTracker implements AutoCloseable public void close() { } - - /** - * A base instance of a MetricsContext. Classes extending this class should exhibit the - * behavior of "starting" a timer upon construction, and "stopping" the timer when the - * {@link MetricsTimerContext#stop()} method is called. - * - * @author Brett Wooldridge - */ - public static class MetricsTimerContext - { - public void stop() - { - // do nothing - } - } } diff --git a/src/main/java/com/zaxxer/hikari/metrics/dropwizard/CodaHaleMetricsTracker.java b/src/main/java/com/zaxxer/hikari/metrics/dropwizard/CodaHaleMetricsTracker.java index 99a58f73..f54dd543 100644 --- a/src/main/java/com/zaxxer/hikari/metrics/dropwizard/CodaHaleMetricsTracker.java +++ b/src/main/java/com/zaxxer/hikari/metrics/dropwizard/CodaHaleMetricsTracker.java @@ -16,6 +16,8 @@ package com.zaxxer.hikari.metrics.dropwizard; +import java.util.concurrent.TimeUnit; + import com.codahale.metrics.Gauge; import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; @@ -84,16 +86,16 @@ public final class CodaHaleMetricsTracker extends MetricsTracker /** {@inheritDoc} */ @Override - public Context recordConnectionRequest() + public void recordConnectionAcquireNanos(final long elapsedAcquireNanos) { - return new Context(connectionObtainTimer); + connectionObtainTimer.update(elapsedAcquireNanos, TimeUnit.NANOSECONDS); } /** {@inheritDoc} */ @Override - public void recordConnectionUsage(final long elapsedLastBorrowed) + public void recordConnectionUsageMillis(final long elapsedBorrowedMilli) { - connectionUsage.update(elapsedLastBorrowed); + connectionUsage.update(elapsedBorrowedMilli); } public Timer getConnectionAcquisitionTimer() @@ -105,20 +107,4 @@ public final class CodaHaleMetricsTracker extends MetricsTracker { return connectionUsage; } - - public static final class Context extends MetricsTimerContext - { - final Timer.Context innerContext; - - Context(Timer timer) { - innerContext = timer.time(); - } - - /** {@inheritDoc} */ - @Override - public void stop() - { - innerContext.stop(); - } - } } diff --git a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java index 84a1482d..f7a1edda 100644 --- a/src/main/java/com/zaxxer/hikari/pool/HikariPool.java +++ b/src/main/java/com/zaxxer/hikari/pool/HikariPool.java @@ -43,8 +43,6 @@ import com.codahale.metrics.MetricRegistry; import com.codahale.metrics.health.HealthCheckRegistry; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariPoolMXBean; -import com.zaxxer.hikari.metrics.MetricsTracker; -import com.zaxxer.hikari.metrics.MetricsTracker.MetricsTimerContext; import com.zaxxer.hikari.metrics.MetricsTrackerFactory; import com.zaxxer.hikari.metrics.PoolStats; import com.zaxxer.hikari.metrics.dropwizard.CodahaleHealthChecker; @@ -158,7 +156,6 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL try { long timeout = hardTimeout; - final MetricsTimerContext metricsContext = (isRecordMetrics ? metricsTracker.recordConnectionRequest() : MetricsTracker.NO_CONTEXT); do { final PoolEntry poolEntry = connectionBag.borrow(timeout, TimeUnit.MILLISECONDS); if (poolEntry == null) { @@ -171,8 +168,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL timeout = hardTimeout - clockSource.elapsedMillis(startTime, now); } else { - metricsTracker.recordLastBurrowed(poolEntry, now); - metricsContext.stop(); + metricsTracker.recordBorrowStats(poolEntry, startTime); return poolEntry.createProxyConnection(leakTask.start(poolEntry), now); } } diff --git a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java index 127d52eb..2168f9f9 100644 --- a/src/main/java/com/zaxxer/hikari/pool/PoolBase.java +++ b/src/main/java/com/zaxxer/hikari/pool/PoolBase.java @@ -23,7 +23,7 @@ import org.slf4j.LoggerFactory; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.metrics.MetricsTracker; -import com.zaxxer.hikari.metrics.MetricsTracker.MetricsTimerContext; +import com.zaxxer.hikari.util.ClockSource; import com.zaxxer.hikari.util.DefaultThreadFactory; import com.zaxxer.hikari.util.DriverDataSource; import com.zaxxer.hikari.util.PropertyElf; @@ -564,33 +564,26 @@ abstract class PoolBase tracker.close(); } - MetricsTimerContext recordConnectionRequest() - { - return tracker.recordConnectionRequest(); - } - void recordConnectionUsage(final PoolEntry poolEntry) { - tracker.recordConnectionUsage(poolEntry.getElapsedLastBorrowed()); + tracker.recordConnectionUsageMillis(poolEntry.getElapsedLastBorrowed()); } /** * @param poolEntry * @param now */ - void recordLastBurrowed(final PoolEntry poolEntry, final long now) + void recordBorrowStats(final PoolEntry poolEntry, final long startTime) { + final long now = ClockSource.INSTANCE.currentTime(); poolEntry.lastBorrowed = now; + tracker.recordConnectionAcquireNanos(ClockSource.INSTANCE.elapsedNanos(startTime, now)); } } static final class NopMetricsTrackerDelegate extends MetricsTrackerDelegate { - MetricsTimerContext recordConnectionRequest() - { - return MetricsTracker.NO_CONTEXT; - } - + @Override void recordConnectionUsage(final PoolEntry poolEntry) { // no-op @@ -603,7 +596,7 @@ abstract class PoolBase } @Override - void recordLastBurrowed(final PoolEntry poolEntry, final long now) + void recordBorrowStats(final PoolEntry poolEntry, final long startTime) { // no-op } diff --git a/src/main/java/com/zaxxer/hikari/util/ClockSource.java b/src/main/java/com/zaxxer/hikari/util/ClockSource.java index 07dae683..1cca766c 100644 --- a/src/main/java/com/zaxxer/hikari/util/ClockSource.java +++ b/src/main/java/com/zaxxer/hikari/util/ClockSource.java @@ -63,6 +63,25 @@ public interface ClockSource */ long elapsedMillis(long startTime, long endTime); + /** + * Convert an opaque time-stamp returned by currentTime() into an + * elapsed time in milliseconds, based on the current instant in time. + * + * @param startTime an opaque time-stamp returned by an instance of this class + * @return the elapsed time between startTime and now in milliseconds + */ + long elapsedNanos(long startTime); + + /** + * Get the difference in nanoseconds between two opaque time-stamps returned + * by currentTime(). + * + * @param startTime an opaque time-stamp returned by an instance of this class + * @param endTime an opaque time-stamp returned by an instance of this class + * @return the elapsed time between startTime and endTime in nanoseconds + */ + long elapsedNanos(long startTime, long endTime); + /** * Return the specified opaque time-stamp plus the specified number of milliseconds. * @@ -72,6 +91,12 @@ public interface ClockSource */ long plusMillis(long time, long millis); + /** + * Get the TimeUnit the ClockSource is denominated in. + * @return + */ + TimeUnit getSourceTimeUnit(); + /** * Factory class used to create a platform-specific ClockSource. */ @@ -113,17 +138,38 @@ public interface ClockSource /** {@inheritDoc} */ @Override - public long toMillis(long time) + public long elapsedNanos(final long startTime) + { + return TimeUnit.MILLISECONDS.toNanos(System.currentTimeMillis() - startTime); + } + + /** {@inheritDoc} */ + @Override + public long elapsedNanos(final long startTime, final long endTime) + { + return TimeUnit.MILLISECONDS.toNanos(endTime - startTime); + } + + /** {@inheritDoc} */ + @Override + public long toMillis(final long time) { return time; } /** {@inheritDoc} */ @Override - public long plusMillis(long time, long millis) + public long plusMillis(final long time, final long millis) { return time + millis; } + + /** {@inheritDoc} */ + @Override + public TimeUnit getSourceTimeUnit() + { + return TimeUnit.MILLISECONDS; + } } final class NanosecondClockSource implements ClockSource @@ -156,11 +202,32 @@ public interface ClockSource return TimeUnit.NANOSECONDS.toMillis(endTime - startTime); } + /** {@inheritDoc} */ + @Override + public long elapsedNanos(final long startTime) + { + return System.nanoTime() - startTime; + } + + /** {@inheritDoc} */ + @Override + public long elapsedNanos(final long startTime, final long endTime) + { + return endTime - startTime; + } + /** {@inheritDoc} */ @Override public long plusMillis(final long time, final long millis) { return time + TimeUnit.MILLISECONDS.toNanos(millis); } + + /** {@inheritDoc} */ + @Override + public TimeUnit getSourceTimeUnit() + { + return TimeUnit.NANOSECONDS; + } } } diff --git a/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java b/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java index 4c446954..a835e653 100644 --- a/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java +++ b/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java @@ -121,18 +121,16 @@ public class ConcurrentBag implements AutoCloseab public T borrow(long timeout, final TimeUnit timeUnit) throws InterruptedException { // Try the thread-local list first, if there are no blocked threads waiting already - if (!synchronizer.hasQueuedThreads()) { - List list = threadList.get(); - if (weakThreadLocals && list == null) { - list = new ArrayList<>(16); - threadList.set(list); - } + List list = threadList.get(); + if (weakThreadLocals && list == null) { + list = new ArrayList<>(16); + threadList.set(list); + } - for (int i = list.size() - 1; i >= 0; i--) { - final T bagEntry = (T) (weakThreadLocals ? ((WeakReference) list.remove(i)).get() : list.remove(i)); - if (bagEntry != null && bagEntry.compareAndSet(STATE_NOT_IN_USE, STATE_IN_USE)) { - return bagEntry; - } + for (int i = list.size() - 1; i >= 0; i--) { + final T bagEntry = (T) (weakThreadLocals ? ((WeakReference) list.remove(i)).get() : list.remove(i)); + if (bagEntry != null && bagEntry.compareAndSet(STATE_NOT_IN_USE, STATE_IN_USE)) { + return bagEntry; } }