From 30c6e7409c10ebef3d1f1b321cea6f55d49d8250 Mon Sep 17 00:00:00 2001 From: Ayush Jaiswal Date: Mon, 17 Jan 2022 23:08:18 +0530 Subject: [PATCH] #5842 stopTimers capture in ServiceManager --- .../util/concurrent/ServiceManagerTest.java | 12 ++ .../util/concurrent/ServiceManager.java | 124 +++++++++++++++--- 2 files changed, 118 insertions(+), 18 deletions(-) diff --git a/guava-tests/test/com/google/common/util/concurrent/ServiceManagerTest.java b/guava-tests/test/com/google/common/util/concurrent/ServiceManagerTest.java index e8552eda99b9..e78d74db0607 100644 --- a/guava-tests/test/com/google/common/util/concurrent/ServiceManagerTest.java +++ b/guava-tests/test/com/google/common/util/concurrent/ServiceManagerTest.java @@ -144,6 +144,18 @@ public void testServiceStartupDurations() { assertThat(startupTimes.get(b)).isAtLeast(Duration.ofMillis(353)); } + public void testServiceStopDurations() { + Service a = new NoOpDelayedService(150); + Service b = new NoOpDelayedService(353); + ServiceManager serviceManager = new ServiceManager(asList(a, b)); + serviceManager.startAsync().awaitHealthy(); + serviceManager.stopAsync().awaitStopped(); + ImmutableMap stopDurations = serviceManager.stopDurations(); + assertThat(stopDurations).hasSize(2); + assertThat(stopDurations.get(a)).isAtLeast(Duration.ofMillis(150)); + assertThat(stopDurations.get(b)).isAtLeast(Duration.ofMillis(353)); + } + public void testServiceStartupTimes_selfStartingServices() { // This tests to ensure that: diff --git a/guava/src/com/google/common/util/concurrent/ServiceManager.java b/guava/src/com/google/common/util/concurrent/ServiceManager.java index 5ab95ffa6b74..c5ab72fcf5c4 100644 --- a/guava/src/com/google/common/util/concurrent/ServiceManager.java +++ b/guava/src/com/google/common/util/concurrent/ServiceManager.java @@ -55,10 +55,7 @@ import com.google.j2objc.annotations.WeakOuter; import java.lang.ref.WeakReference; import java.time.Duration; -import java.util.Collections; -import java.util.EnumSet; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -334,6 +331,7 @@ public void awaitHealthy(long timeout, TimeUnit unit) throws TimeoutException { @CanIgnoreReturnValue public ServiceManager stopAsync() { for (Service service : services) { + state.tryStartStopTiming(service); service.stopAsync(); } return this; @@ -414,6 +412,17 @@ public ImmutableMap startupTimes() { return state.startupTimes(); } + /** + * Returns the service stop times. This value will only return stop times for services that + * have finished stopping. + * + * @return Map of services and their corresponding stop time in millis, the map entries will be + * ordered by stop time. + */ + public ImmutableMap stopTimes() { + return state.stopTimes(); + } + /** * Returns the service load times. This value will only return startup times for services that * have finished starting. @@ -428,6 +437,19 @@ public ImmutableMap startupDurations() { Maps.transformValues(startupTimes(), Duration::ofMillis)); } + /** + * Returns the service stopping termination times. This method returns values for only those services + * that have finished stopping. + * + * @return Map of services and their corresponding stop time, the map entries will be ordered + * by stop time. + */ + @J2ObjCIncompatible + public ImmutableMap stopDurations() { + return ImmutableMap.copyOf( + Maps.transformValues(stopTimes(), Duration::ofMillis)); + } + @Override public String toString() { return MoreObjects.toStringHelper(ServiceManager.class) @@ -452,6 +474,9 @@ private static final class ServiceManagerState { @GuardedBy("monitor") final Map startupTimers = Maps.newIdentityHashMap(); + @GuardedBy("monitor") + final Map stopTimers = new IdentityHashMap<>(); + /** * These two booleans are used to mark the state as ready to start. * @@ -542,6 +567,22 @@ void tryStartTiming(Service service) { } } + /** + * Attempts to start the stop timer immediately prior to the service being stopped via {@link + * Service#stopAsync()} + */ + void tryStartStopTiming(Service service) { + monitor.enter(); + try { + Stopwatch stopwatch = stopTimers.get(service); + if (stopwatch == null) { + stopTimers.put(service, Stopwatch.createStarted()); + } + } finally { + monitor.leave(); + } + } + /** * Marks the {@link State} as ready to receive transitions. Returns true if no transitions have * been observed yet. @@ -661,6 +702,35 @@ public Long apply(Entry input) { return ImmutableMap.copyOf(loadTimes); } + ImmutableMap stopTimes() { + List> stopTimes; + monitor.enter(); + try { + stopTimes = Lists.newArrayListWithCapacity(stopTimers.size()); + // N.B. There will only be an entry in the map if the service has stopped + for (Entry entry : stopTimers.entrySet()) { + Service service = entry.getKey(); + Stopwatch stopwatch = entry.getValue(); + if (!stopwatch.isRunning() && !(service instanceof NoOpService)) { + stopTimes.add(Maps.immutableEntry(service, stopwatch.elapsed(MILLISECONDS))); + } + } + } finally { + monitor.leave(); + } + Collections.sort( + stopTimes, + Ordering.natural() + .onResultOf( + new Function, Long>() { + @Override + public Long apply(Entry input) { + return input.getValue(); + } + })); + return ImmutableMap.copyOf(stopTimes); + } + /** * Updates the state with the given service transition. * @@ -693,20 +763,8 @@ void transitionService(final Service service, State from, State to) { "Service %s in the state map unexpectedly at %s", service, to); - // Update the timer - Stopwatch stopwatch = startupTimers.get(service); - if (stopwatch == null) { - // This means the service was started by some means other than ServiceManager.startAsync - stopwatch = Stopwatch.createStarted(); - startupTimers.put(service, stopwatch); - } - if (to.compareTo(RUNNING) >= 0 && stopwatch.isRunning()) { - // N.B. if we miss the STARTING event then we may never record a startup time. - stopwatch.stop(); - if (!(service instanceof NoOpService)) { - logger.log(Level.FINE, "Started {0} in {1}.", new Object[] {service, stopwatch}); - } - } + + updateStartAndStopTimersIfRequired(service, to); // Queue our listeners // Did a service fail? @@ -728,6 +786,36 @@ void transitionService(final Service service, State from, State to) { } } + void updateStartAndStopTimersIfRequired(Service service, State state) { + + // Update the timer + Stopwatch stopwatch = startupTimers.get(service); + if (stopwatch == null) { + // This means the service was started by some means other than ServiceManager.startAsync + stopwatch = Stopwatch.createStarted(); + startupTimers.put(service, stopwatch); + } + if (state.compareTo(RUNNING) >= 0 && stopwatch.isRunning()) { + // N.B. if we miss the STARTING event then we may never record a startup time. + stopwatch.stop(); + if (!(service instanceof NoOpService)) { + logger.log(Level.FINE, "Started {0} in {1}.", new Object[]{service, stopwatch}); + } + } + + stopwatch = stopTimers.get(service); + // can be null only if the service has been triggered stop from somewhere else than ServiceManager.stopAsync + if (stopwatch == null) { + stopTimers.put(service, Stopwatch.createStarted()); + } + if (state.compareTo(TERMINATED) >= 0 && stopwatch.isRunning()) { + stopwatch.stop(); + if (!(service instanceof NoOpService)) { + logger.log(Level.FINE, "Stopped {0} in {1}.", new Object[]{service, stopwatch}); + } + } + } + void enqueueStoppedEvent() { listeners.enqueue(STOPPED_EVENT); }