diff --git a/ebean-api/src/main/java/io/ebean/meta/AbstractMetricVisitor.java b/ebean-api/src/main/java/io/ebean/meta/AbstractMetricVisitor.java index 51df586d9d..3619ba665a 100644 --- a/ebean-api/src/main/java/io/ebean/meta/AbstractMetricVisitor.java +++ b/ebean-api/src/main/java/io/ebean/meta/AbstractMetricVisitor.java @@ -5,13 +5,18 @@ */ public abstract class AbstractMetricVisitor implements MetricVisitor { - private final boolean reset; + private final Mode mode; private final boolean collectTransactionMetrics; private final boolean collectQueryMetrics; private final boolean collectL2Metrics; public AbstractMetricVisitor(boolean reset, boolean collectTransactionMetrics, boolean collectQueryMetrics, boolean collectL2Metrics) { - this.reset = reset; + this(reset ? Mode.RESET : Mode.CUMULATIVE, + collectTransactionMetrics, collectQueryMetrics, collectL2Metrics); + } + + public AbstractMetricVisitor(Mode mode, boolean collectTransactionMetrics, boolean collectQueryMetrics, boolean collectL2Metrics) { + this.mode = mode; this.collectTransactionMetrics = collectTransactionMetrics; this.collectQueryMetrics = collectQueryMetrics; this.collectL2Metrics = collectL2Metrics; @@ -19,7 +24,12 @@ public AbstractMetricVisitor(boolean reset, boolean collectTransactionMetrics, b @Override public boolean reset() { - return reset; + return mode == Mode.RESET; + } + + @Override + public Mode mode() { + return mode; } @Override @@ -47,4 +57,3 @@ public void visitEnd() { // do nothing by default } } - diff --git a/ebean-api/src/main/java/io/ebean/meta/BasicMetricVisitor.java b/ebean-api/src/main/java/io/ebean/meta/BasicMetricVisitor.java index 653068a981..d8a7d176e3 100644 --- a/ebean-api/src/main/java/io/ebean/meta/BasicMetricVisitor.java +++ b/ebean-api/src/main/java/io/ebean/meta/BasicMetricVisitor.java @@ -30,7 +30,16 @@ public BasicMetricVisitor(String name, Function naming) { * Construct specifying reset and what to collect. */ public BasicMetricVisitor(String name, Function naming, boolean reset, boolean collectTransactionMetrics, boolean collectQueryMetrics, boolean collectL2Metrics) { - super(reset, collectTransactionMetrics, collectQueryMetrics, collectL2Metrics); + this(name, naming, reset ? Mode.RESET : Mode.CUMULATIVE, + collectTransactionMetrics, collectQueryMetrics, collectL2Metrics); + } + + /** + * Construct specifying the collection mode and what to collect. + */ + public BasicMetricVisitor(String name, Function naming, Mode mode, + boolean collectTransactionMetrics, boolean collectQueryMetrics, boolean collectL2Metrics) { + super(mode, collectTransactionMetrics, collectQueryMetrics, collectL2Metrics); this.name = name; this.naming = naming; } diff --git a/ebean-api/src/main/java/io/ebean/meta/MetricVisitor.java b/ebean-api/src/main/java/io/ebean/meta/MetricVisitor.java index 519c13d8f9..f07839644b 100644 --- a/ebean-api/src/main/java/io/ebean/meta/MetricVisitor.java +++ b/ebean-api/src/main/java/io/ebean/meta/MetricVisitor.java @@ -7,6 +7,12 @@ */ public interface MetricVisitor { + enum Mode { + RESET, + CUMULATIVE, + DELTA + } + /** * Return the naming convention that should be applied to the reported metric names. */ @@ -17,6 +23,13 @@ public interface MetricVisitor { */ boolean reset(); + /** + * Return the metric collection mode. + */ + default Mode mode() { + return reset() ? Mode.RESET : Mode.CUMULATIVE; + } + /** * Return true if we should visit the transaction metrics. */ diff --git a/ebean-api/src/main/java/io/ebean/metric/TimedMetric.java b/ebean-api/src/main/java/io/ebean/metric/TimedMetric.java index 17c3defb95..96749f873e 100644 --- a/ebean-api/src/main/java/io/ebean/metric/TimedMetric.java +++ b/ebean-api/src/main/java/io/ebean/metric/TimedMetric.java @@ -37,6 +37,16 @@ public interface TimedMetric { */ TimedMetricStats collect(boolean reset); + /** + * Collect a snapshot using the given collection mode. + * + *

Implementations that do not support delta collection use cumulative + * collection for {@link MetricVisitor.Mode#DELTA}.

+ */ + default TimedMetricStats collect(MetricVisitor.Mode mode) { + return collect(mode == MetricVisitor.Mode.RESET); + } + /** * Visit non empty metrics. */ diff --git a/ebean-api/src/test/java/io/ebean/meta/MetaInfoManagerTest.java b/ebean-api/src/test/java/io/ebean/meta/MetaInfoManagerTest.java index 607d07d75a..0de98a0670 100644 --- a/ebean-api/src/test/java/io/ebean/meta/MetaInfoManagerTest.java +++ b/ebean-api/src/test/java/io/ebean/meta/MetaInfoManagerTest.java @@ -43,4 +43,18 @@ public List queryPlanCollectNow(QueryPlanRequest request) { assertThat(manager.collectMetrics(false)).isSameAs(metrics); } + + @Test + void basicMetricVisitorSupportsExplicitCollectionModes() { + var reset = new BasicMetricVisitor("db", MetricNamingMatch.INSTANCE, MetricVisitor.Mode.RESET, true, true, true); + var cumulative = new BasicMetricVisitor("db", MetricNamingMatch.INSTANCE, MetricVisitor.Mode.CUMULATIVE, true, true, true); + var delta = new BasicMetricVisitor("db", MetricNamingMatch.INSTANCE, MetricVisitor.Mode.DELTA, true, true, true); + + assertThat(reset.reset()).isTrue(); + assertThat(reset.mode()).isEqualTo(MetricVisitor.Mode.RESET); + assertThat(cumulative.reset()).isFalse(); + assertThat(cumulative.mode()).isEqualTo(MetricVisitor.Mode.CUMULATIVE); + assertThat(delta.reset()).isFalse(); + assertThat(delta.mode()).isEqualTo(MetricVisitor.Mode.DELTA); + } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DCountMetric.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DCountMetric.java index 34c47cfc64..05db9fd386 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DCountMetric.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DCountMetric.java @@ -4,24 +4,19 @@ import io.ebean.metric.CountMetric; import io.ebean.metric.CountMetricStats; -import java.util.concurrent.atomic.LongAdder; - /** * Used to collect counter metrics. */ final class DCountMetric implements CountMetric { private final String name; - private final LongAdder count = new LongAdder(); + private final ValueAdder count = new ValueAdder(); private String reportName; DCountMetric(String name) { this.name = name; } - /** - * Add a value. Usually the value is Time or Bytes etc. - */ @Override public void add(long value) { count.add(value); @@ -29,12 +24,12 @@ public void add(long value) { @Override public void increment() { - count.increment(); + count.add(1); } @Override public boolean isEmpty() { - return count.sum() == 0; + return count.currentValue() == 0; } @Override @@ -44,12 +39,25 @@ public void reset() { @Override public long get(boolean reset) { - return reset ? count.sumThenReset() : count.sum(); + return reset ? count.getAndReset() : count.cumulative(); } @Override public void visit(MetricVisitor visitor) { - long val = visitor.reset() ? count.sumThenReset() : count.sum(); + long val; + switch (visitor.mode()) { + case RESET: + val = count.getAndReset(); + break; + case CUMULATIVE: + val = count.cumulative(); + break; + case DELTA: + val = count.delta(); + break; + default: + throw new IllegalStateException("Unknown metric collection mode"); + } if (val > 0) { final String name = reportName != null ? reportName : reportName(visitor); visitor.visitCount(new DCountMetricStats(name, val)); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java index 183a5d457e..e8a6a7ae54 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DQueryPlanMetric.java @@ -20,7 +20,7 @@ final class DQueryPlanMetric implements QueryPlanMetric { @Override public void visit(MetricVisitor visitor) { - TimedMetricStats stats = metric.collect(visitor.reset()); + TimedMetricStats stats = metric.collect(visitor.mode()); if (stats != null) { String name = reportName != null ? reportName : reportName(visitor); visitor.visitQuery(new Stats(name, meta, stats, collected)); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedMetric.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedMetric.java index 745a8efd83..619f9f9218 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedMetric.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedMetric.java @@ -4,7 +4,6 @@ import io.ebean.metric.TimedMetric; import java.util.concurrent.atomic.LongAccumulator; -import java.util.concurrent.atomic.LongAdder; /** * Used to collect timed execution statistics. @@ -15,8 +14,8 @@ final class DTimedMetric implements TimedMetric { private final String name; - private final LongAdder count = new LongAdder(); - private final LongAdder total = new LongAdder(); + private final ValueAdder count = new ValueAdder(); + private final ValueAdder total = new ValueAdder(); private final LongAccumulator max = new LongAccumulator(Math::max, 0); private boolean collected; private String reportName; @@ -43,14 +42,14 @@ public void addSinceNanos(long startNanos) { @Override public void add(long value) { - count.increment(); + count.add(1); total.add(value); max.accumulate(value); } @Override public boolean isEmpty() { - return count.sum() == 0; + return count.currentValue() == 0; } @Override @@ -62,29 +61,61 @@ public void reset() { @Override public void visit(MetricVisitor visitor) { - final long countSum = visitor.reset() ? count.sumThenReset() : count.sum(); - if (countSum > 0) { + final DTimeMetricStats stats = collect(visitor.mode()); + if (stats != null) { final String name = reportName != null ? reportName : reportName(visitor); - visitor.visitTimed(stats(visitor.reset(), name, countSum)); + stats.setName(name); + visitor.visitTimed(stats); } } @Override public DTimeMetricStats collect(boolean reset) { - final long countSum = reset ? count.sumThenReset() : count.sum(); + return collect(reset ? MetricVisitor.Mode.RESET : MetricVisitor.Mode.CUMULATIVE); + } + + @Override + public DTimeMetricStats collect(MetricVisitor.Mode mode) { + final long countSum; + switch (mode) { + case RESET: + countSum = count.getAndReset(); + break; + case CUMULATIVE: + countSum = count.cumulative(); + break; + case DELTA: + countSum = count.delta(); + break; + default: + throw new IllegalStateException("Unknown metric collection mode"); + } if (countSum == 0) { return null; } else { - return stats(reset, name, countSum); + return stats(mode, name, countSum); } } /** * Return the current statistics resetting the internal values if reset is true. */ - private DTimeMetricStats stats(boolean reset, String name, long countSum) { + private DTimeMetricStats stats(MetricVisitor.Mode mode, String name, long countSum) { try { - final long totalSum = reset ? total.sumThenReset() : total.sum(); + final long totalSum; + switch (mode) { + case RESET: + totalSum = total.getAndReset(); + break; + case CUMULATIVE: + totalSum = total.cumulative(); + break; + case DELTA: + totalSum = total.delta(); + break; + default: + throw new IllegalStateException("Unknown metric collection mode"); + } return new DTimeMetricStats(name, collected, countSum, totalSum, max.getThenReset()); } finally { collected = true; diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedProfileLocation.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedProfileLocation.java index fcb7c2f829..25ee3899fd 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedProfileLocation.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/DTimedProfileLocation.java @@ -46,7 +46,7 @@ public void add(long executionTime) { @Override public void visit(MetricVisitor visitor) { - TimedMetricStats collect = timedMetric.collect(visitor.reset()); + TimedMetricStats collect = timedMetric.collect(visitor.mode()); if (collect != null) { final String name = reportName != null ? reportName : reportName(visitor, collect.name()); collect.setName(name); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/profile/ValueAdder.java b/ebean-core/src/main/java/io/ebeaninternal/server/profile/ValueAdder.java new file mode 100644 index 0000000000..74fd5acf6b --- /dev/null +++ b/ebean-core/src/main/java/io/ebeaninternal/server/profile/ValueAdder.java @@ -0,0 +1,42 @@ +package io.ebeaninternal.server.profile; + +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.LongAdder; + +/** + * Accumulates a value while supporting cumulative and reset-based delta reads. + */ +final class ValueAdder { + + private final LongAdder value = new LongAdder(); + private final AtomicLong previousValue = new AtomicLong(); + + void add(long amount) { + value.add(amount); + } + + long cumulative() { + return value.sum(); + } + + long delta() { + long currentValue = value.sum(); + long previous = previousValue.getAndSet(currentValue); + return currentValue >= previous ? currentValue - previous : currentValue; + } + + long getAndReset() { + long currentValue = value.sumThenReset(); + previousValue.set(0); + return currentValue; + } + + void reset() { + value.reset(); + previousValue.set(0); + } + + long currentValue() { + return value.sum(); + } +} diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/profile/DCountMetricTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/profile/DCountMetricTest.java index 0abf646663..7ae4ccfe66 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/profile/DCountMetricTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/profile/DCountMetricTest.java @@ -2,6 +2,7 @@ import io.ebean.meta.BasicMetricVisitor; import io.ebean.meta.MetaCountMetric; +import io.ebean.meta.MetricVisitor; import org.junit.jupiter.api.Test; import java.util.List; @@ -40,4 +41,61 @@ void visit() { assertThat(result2.get(0).count()).isEqualTo(12); } } + + @Test + void cumulativeAndDeltaAreIndependent() { + DCountMetric counter = new DCountMetric("org.hello"); + counter.add(7); + + assertThat(counter.get(false)).isEqualTo(7); + assertThat(counter.get(false)).isEqualTo(7); + assertThat(counter.get(true)).isEqualTo(7); + + counter.add(5); + assertThat(counter.get(false)).isEqualTo(5); + assertThat(counter.get(true)).isEqualTo(5); + assertThat(counter.get(true)).isEqualTo(0); + } + + @Test + void valueAdderSupportsExplicitCollectionOperations() { + var values = new ValueAdder(); + values.add(7); + + assertThat(values.cumulative()).isEqualTo(7); + assertThat(values.delta()).isEqualTo(7); + + values.add(5); + assertThat(values.cumulative()).isEqualTo(12); + assertThat(values.delta()).isEqualTo(5); + assertThat(values.getAndReset()).isEqualTo(12); + assertThat(values.cumulative()).isEqualTo(0); + assertThat(values.delta()).isEqualTo(0); + } + + @Test + void visitorCanCollectDeltaWithoutResettingCumulativeValue() { + var counter = new DCountMetric("org.hello"); + counter.add(7); + + var cumulative = new BasicMetricVisitor("db", naming, MetricVisitor.Mode.CUMULATIVE, true, true, true); + counter.visit(cumulative); + assertThat(cumulative.countMetrics()).hasSize(1); + assertThat(cumulative.countMetrics().get(0).count()).isEqualTo(7); + + var delta = new BasicMetricVisitor("db", naming, MetricVisitor.Mode.DELTA, true, true, true); + counter.visit(delta); + assertThat(delta.countMetrics()).hasSize(1); + assertThat(delta.countMetrics().get(0).count()).isEqualTo(7); + + counter.add(5); + delta = new BasicMetricVisitor("db", naming, MetricVisitor.Mode.DELTA, true, true, true); + counter.visit(delta); + assertThat(delta.countMetrics()).hasSize(1); + assertThat(delta.countMetrics().get(0).count()).isEqualTo(5); + + cumulative = new BasicMetricVisitor("db", naming, MetricVisitor.Mode.CUMULATIVE, true, true, true); + counter.visit(cumulative); + assertThat(cumulative.countMetrics().get(0).count()).isEqualTo(12); + } } diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/profile/DTimedMetricTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/profile/DTimedMetricTest.java index 103b3b42c3..d6629a0a2d 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/profile/DTimedMetricTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/profile/DTimedMetricTest.java @@ -116,4 +116,24 @@ void collectCumulativeResetsMax() { assertThat(stats.total()).isEqualTo(1470); assertThat(stats.max()).isEqualTo(160); } + + @Test + void cumulativeAndDeltaAreIndependent() { + DTimedMetric metric = new DTimedMetric("org.timed"); + metric.add(560); + metric.add(500); + + DTimeMetricStats cumulative = metric.collect(false); + assertThat(cumulative.count()).isEqualTo(2); + assertThat(cumulative.total()).isEqualTo(1060); + + metric.add(160); + + DTimeMetricStats delta = metric.collect(true); + assertThat(delta.count()).isEqualTo(3); + assertThat(delta.total()).isEqualTo(1220); + + cumulative = metric.collect(false); + assertThat(cumulative).isNull(); + } } diff --git a/ebean-test/src/test/java/org/tests/model/basic/cache/TestNatKeyCacheWithForeignKey.java b/ebean-test/src/test/java/org/tests/model/basic/cache/TestNatKeyCacheWithForeignKey.java index c08edb848f..806d4a7cbc 100644 --- a/ebean-test/src/test/java/org/tests/model/basic/cache/TestNatKeyCacheWithForeignKey.java +++ b/ebean-test/src/test/java/org/tests/model/basic/cache/TestNatKeyCacheWithForeignKey.java @@ -30,6 +30,7 @@ public void test_findOne() { setupData(); clearAllL2Cache(); + getStats(); final OCachedAppDetail found0 = findDetail(app0, "detail0"); assertThat(found0).isNotNull();