From a2d25c5082b7667f7261e23aa2601c1e00ed357d Mon Sep 17 00:00:00 2001 From: Olli Virtanen Date: Thu, 7 Jun 2018 17:09:06 +0200 Subject: Switched to Instant/Duration for time, test case cleanup --- .../jdisc/metric/GarbageCollectionMetrics.java | 32 ++++++++++++---------- .../jdisc/metric/GarbageCollectionMetricsTest.java | 27 ++++++++---------- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java index c208da6a07d..317cd3134af 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java @@ -6,6 +6,7 @@ import java.lang.management.GarbageCollectorMXBean; import java.lang.management.ManagementFactory; import java.time.Clock; import java.time.Duration; +import java.time.Instant; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; @@ -15,21 +16,22 @@ import java.util.Map; * @author ollivir */ public class GarbageCollectionMetrics { - private static final String GC_COUNT = "jdisc.gc.count"; - private static final String GC_TIME = "jdisc.gc.ms"; + private static final String GC_PREFIX = "jdisc.gc."; + private static final String GC_COUNT = GC_PREFIX + ".count"; + private static final String GC_TIME = GC_PREFIX + ".ms"; private static final String DIMENSION_KEY = "gcName"; - public static final long REPORTING_INTERVAL = Duration.ofSeconds(62).toMillis(); + public static final Duration REPORTING_INTERVAL = Duration.ofSeconds(62); static class GcStats { - private final long when; + private final Instant when; private final long count; - private final long ms; + private final Duration totalRuntime; - private GcStats(long when, long count, long ms) { + private GcStats(Instant when, long count, Duration totalRuntime) { this.when = when; this.count = count; - this.ms = ms; + this.totalRuntime = totalRuntime; } } @@ -40,26 +42,26 @@ public class GarbageCollectionMetrics { public GarbageCollectionMetrics(Clock clock) { this.clock = clock; this.gcStatistics = new HashMap<>(); - collectGcStatistics(clock.millis()); + collectGcStatistics(clock.instant()); } - private void collectGcStatistics(long now) { + private void collectGcStatistics(Instant now) { for (GarbageCollectorMXBean gcBean : ManagementFactory.getGarbageCollectorMXBeans()) { String gcName = gcBean.getName().replace(" ", ""); - GcStats stats = new GcStats(now, gcBean.getCollectionCount(), gcBean.getCollectionTime()); + GcStats stats = new GcStats(now, gcBean.getCollectionCount(), Duration.ofMillis(gcBean.getCollectionTime())); LinkedList window = gcStatistics.computeIfAbsent(gcName, anyName -> new LinkedList<>()); window.addLast(stats); } } - private void cleanStatistics(long now) { - long oldestToKeep = now - REPORTING_INTERVAL; + private void cleanStatistics(Instant now) { + Instant oldestToKeep = now.minus(REPORTING_INTERVAL); for(Iterator>> it = gcStatistics.entrySet().iterator(); it.hasNext(); ) { Map.Entry> entry = it.next(); LinkedList history = entry.getValue(); - while(history.isEmpty() == false && history.getFirst().when < oldestToKeep) { + while(history.isEmpty() == false && oldestToKeep.isAfter(history.getFirst().when)) { history.removeFirst(); } if(history.isEmpty()) { @@ -69,7 +71,7 @@ public class GarbageCollectionMetrics { } public void emitMetrics(Metric metric) { - long now = clock.millis(); + Instant now = clock.instant(); collectGcStatistics(now); cleanStatistics(now); @@ -82,7 +84,7 @@ public class GarbageCollectionMetrics { Metric.Context gcContext = metric.createContext(contextData); metric.set(GC_COUNT, latest.count - reference.count, gcContext); - metric.set(GC_TIME, latest.ms - reference.ms, gcContext); + metric.set(GC_TIME, latest.totalRuntime.minus(reference.totalRuntime).toMillis(), gcContext); } } diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java index 76ad430f4b9..61d8763b852 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java @@ -1,55 +1,52 @@ package com.yahoo.container.jdisc.metric; import com.yahoo.jdisc.Metric; +import com.yahoo.test.ManualClock; import org.junit.Test; import java.lang.management.ManagementFactory; -import java.time.Clock; +import java.time.Duration; import java.util.LinkedList; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; /** * @author ollivir */ public class GarbageCollectionMetricsTest { @Test - public void gc_metrics_reported_at_intervals() { - Clock clock = mock(Clock.class); + public void gc_metrics_are_collected_in_a_sliding_window() { + ManualClock clock = new ManualClock(); Metric metric = mock(Metric.class); - int base = ManagementFactory.getGarbageCollectorMXBeans().size(); + int garbageCollectors = ManagementFactory.getGarbageCollectorMXBeans().size(); - long iv = GarbageCollectionMetrics.REPORTING_INTERVAL; - when(clock.millis()).thenReturn(10L); + Duration interval = GarbageCollectionMetrics.REPORTING_INTERVAL; GarbageCollectionMetrics garbageCollectionMetrics = new GarbageCollectionMetrics(clock); - assertThat(garbageCollectionMetrics.getGcStatistics().keySet().size(), is(base)); + assertThat(garbageCollectionMetrics.getGcStatistics().keySet().size(), is(garbageCollectors)); - when(clock.millis()).thenReturn(iv); + clock.advance(interval.minus(Duration.ofMillis(10))); garbageCollectionMetrics.emitMetrics(metric); assertWindowLengths(garbageCollectionMetrics, 2); - when(clock.millis()).thenReturn(10L + iv); + clock.advance(Duration.ofMillis(10)); garbageCollectionMetrics.emitMetrics(metric); assertWindowLengths(garbageCollectionMetrics, 3); - when(clock.millis()).thenReturn(20L + iv); + clock.advance(Duration.ofMillis(10)); garbageCollectionMetrics.emitMetrics(metric); assertWindowLengths(garbageCollectionMetrics, 3); - when(clock.millis()).thenReturn(20L + iv + iv); + clock.advance(interval); garbageCollectionMetrics.emitMetrics(metric); assertWindowLengths(garbageCollectionMetrics, 2); - verify(metric, times(base * 4 * 2)).set(anyString(), any(), any()); + verify(metric, times(garbageCollectors * 4 * 2)).set(anyString(), any(), any()); } private static void assertWindowLengths(GarbageCollectionMetrics gcm, int count) { -- cgit v1.2.3