Skip to content

Commit 88b853a

Browse files
authored
metric: improve accuracy of timer metric under contention (#18219)
* metric: improve accuracy of timer metric excludes contention for recording timing from the timing by locking in the completion time before attempting to record it * remove unused imports
1 parent 75eca84 commit 88b853a

File tree

1 file changed

+22
-19
lines changed

1 file changed

+22
-19
lines changed

logstash-core/src/main/java/org/logstash/instrument/metrics/timer/ConcurrentLiveTimerMetric.java

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,13 @@ protected ConcurrentLiveTimerMetric(final String name) {
4343
@Override
4444
public <T, E extends Throwable> T time(ExceptionalSupplier<T, E> exceptionalSupplier) throws E {
4545
try {
46-
trackedMillisState.getAndUpdate(TrackedMillisState::withIncrementedConcurrency);
46+
trackedMillisState.getAndUpdate(existing -> existing.withIncrementedConcurrency(nanoTimeSupplier.getAsLong()));
4747
return exceptionalSupplier.get();
4848
} finally {
49-
trackedMillisState.getAndUpdate(TrackedMillisState::withDecrementedConcurrency);
49+
// lock in the actual time completed, and resolve state separately
50+
// so that contention for recording state is not included in measurement.
51+
final long endTime = nanoTimeSupplier.getAsLong();
52+
trackedMillisState.getAndUpdate(existing -> existing.withDecrementedConcurrency(endTime));
5053
}
5154
}
5255

@@ -65,13 +68,13 @@ private long getUntrackedMillis() {
6568
}
6669

6770
private long getTrackedMillis() {
68-
return this.trackedMillisState.getAcquire().getValue();
71+
return this.trackedMillisState.getAcquire().getValue(nanoTimeSupplier.getAsLong());
6972
}
7073

7174
interface TrackedMillisState {
72-
TrackedMillisState withIncrementedConcurrency();
73-
TrackedMillisState withDecrementedConcurrency();
74-
long getValue();
75+
TrackedMillisState withIncrementedConcurrency(long asOfNanoTime);
76+
TrackedMillisState withDecrementedConcurrency(long asOfNanoTime);
77+
long getValue(long asOfNanoTime);
7578
}
7679

7780
private class StaticTrackedMillisState implements TrackedMillisState {
@@ -89,18 +92,18 @@ public StaticTrackedMillisState() {
8992
}
9093

9194
@Override
92-
public TrackedMillisState withIncrementedConcurrency() {
93-
return new DynamicTrackedMillisState(nanoTimeSupplier.getAsLong(), this.cumulativeMillis, this.excessNanos, 1);
95+
public TrackedMillisState withIncrementedConcurrency(final long asOfNanoTime) {
96+
return new DynamicTrackedMillisState(asOfNanoTime, this.cumulativeMillis, this.excessNanos, 1);
9497
}
9598

9699
@Override
97-
public TrackedMillisState withDecrementedConcurrency() {
100+
public TrackedMillisState withDecrementedConcurrency(final long asOfNanoTime) {
98101
throw new IllegalStateException("TimerMetrics cannot track negative concurrency");
99102
}
100103

101104

102105
@Override
103-
public long getValue() {
106+
public long getValue(final long asOfNanoTime) {
104107
return cumulativeMillis;
105108
}
106109
}
@@ -122,26 +125,26 @@ private class DynamicTrackedMillisState implements TrackedMillisState {
122125
}
123126

124127
@Override
125-
public TrackedMillisState withIncrementedConcurrency() {
126-
return withAdjustedConcurrency(Vector.INCREMENT);
128+
public TrackedMillisState withIncrementedConcurrency(final long asOfNanoTime) {
129+
return withAdjustedConcurrency(asOfNanoTime, Vector.INCREMENT);
127130
}
128131

129132
@Override
130-
public TrackedMillisState withDecrementedConcurrency() {
131-
return withAdjustedConcurrency(Vector.DECREMENT);
133+
public TrackedMillisState withDecrementedConcurrency(final long asOfNanoTime) {
134+
return withAdjustedConcurrency(asOfNanoTime, Vector.DECREMENT);
132135
}
133136

134137
@Override
135-
public long getValue() {
136-
final long nanoAdjustment = getNanoAdjustment(nanoTimeSupplier.getAsLong());
138+
public long getValue(final long asOfNanoTime) {
139+
final long nanoAdjustment = getNanoAdjustment(asOfNanoTime);
137140
final long milliAdjustment = wholeMillisFromNanos(nanoAdjustment);
138141

139142
return Math.addExact(this.millisAtCheckpoint, milliAdjustment);
140143
}
141144

142-
private TrackedMillisState withAdjustedConcurrency(final Vector concurrencyAdjustmentVector) {
145+
private TrackedMillisState withAdjustedConcurrency(final long asOfNanoTime, final Vector concurrencyAdjustmentVector) {
143146
final int newConcurrency = Math.addExact(this.concurrencySinceCheckpoint, concurrencyAdjustmentVector.value());
144-
final long newCheckpointNanoTime = nanoTimeSupplier.getAsLong();
147+
final long newCheckpointNanoTime = asOfNanoTime;
145148

146149
final long totalNanoAdjustment = getNanoAdjustment(newCheckpointNanoTime);
147150

@@ -165,7 +168,7 @@ private long getNanoAdjustment(final long checkpointNanoTime) {
165168

166169
/**
167170
* This private enum is a type-safety guard for
168-
* {@link DynamicTrackedMillisState#withAdjustedConcurrency(Vector)}.
171+
* {@link DynamicTrackedMillisState#withAdjustedConcurrency(long, Vector)}.
169172
*/
170173
private enum Vector {
171174
INCREMENT{ int value() { return +1; } },

0 commit comments

Comments
 (0)