Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Commit e3bf35f

Browse files
authored
Fix time skew exceptions crashing disruptor thread (#2071)
* Add a fix to metric views when a time skew is detected rather than crashing the application/thread. * Add test for time-rewind issues, and fix problems discovered.
1 parent 6123b84 commit e3bf35f

File tree

2 files changed

+88
-4
lines changed

2 files changed

+88
-4
lines changed

impl_core/src/main/java/io/opencensus/implcore/stats/MutableViewData.java

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ private CumulativeMutableViewData(View view, Timestamp start) {
132132
@javax.annotation.Nullable
133133
@Override
134134
Metric toMetric(Timestamp now, State state) {
135+
handleTimeRewinds(now);
135136
if (state == State.DISABLED) {
136137
return null;
137138
}
@@ -166,6 +167,7 @@ void record(
166167

167168
@Override
168169
ViewData toViewData(Timestamp now, State state) {
170+
handleTimeRewinds(now);
169171
if (state == State.ENABLED) {
170172
return ViewData.create(
171173
super.view,
@@ -180,6 +182,18 @@ ViewData toViewData(Timestamp now, State state) {
180182
}
181183
}
182184

185+
/**
186+
* This method attemps to migrate this view into a reasonable state in the event of time going
187+
* backwards.
188+
*/
189+
private void handleTimeRewinds(Timestamp now) {
190+
if (now.compareTo(start) < 0) {
191+
// Time went backwards, physics is broken, forget what we know.
192+
clearStats();
193+
start = now;
194+
}
195+
}
196+
183197
@Override
184198
void clearStats() {
185199
tagValueAggregationMap.clear();
@@ -304,10 +318,18 @@ private void refreshBucketList(Timestamp now) {
304318
}
305319
Timestamp startOfLastBucket =
306320
CheckerFrameworkUtils.castNonNull(buckets.peekLast()).getStart();
307-
// TODO(songya): decide what to do when time goes backwards
308-
checkArgument(
309-
now.compareTo(startOfLastBucket) >= 0,
310-
"Current time must be within or after the last bucket.");
321+
// Time went backwards! Physics has failed us! drop everything we know and relearn.
322+
// Prioritize: Report data we're confident is correct.
323+
if (now.compareTo(startOfLastBucket) < 0) {
324+
// TODO: configurable time-skew handling with options:
325+
// - Drop events in the future, keep others within a duration.
326+
// - Drop all events on skew
327+
// - Guess at time-skew and "fix" events
328+
// - Reset our "start" time to now if necessary.
329+
buckets.clear();
330+
shiftBucketList(N + 1, now);
331+
return;
332+
}
311333
long elapsedTimeMillis = now.subtractTimestamp(startOfLastBucket).toMillis();
312334
long numOfPadBuckets = elapsedTimeMillis / bucketDuration.toMillis();
313335

impl_core/src/test/java/io/opencensus/implcore/stats/MutableViewDataTest.java

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,19 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020

2121
import io.opencensus.common.Timestamp;
22+
import io.opencensus.implcore.internal.CurrentState;
23+
import io.opencensus.implcore.tags.TagMapImpl;
24+
import io.opencensus.metrics.data.AttachmentValue;
25+
import io.opencensus.stats.Aggregation;
26+
import io.opencensus.stats.Aggregation.Count;
27+
import io.opencensus.stats.Aggregation.Distribution;
28+
import io.opencensus.stats.BucketBoundaries;
29+
import io.opencensus.stats.Measure.MeasureDouble;
30+
import io.opencensus.stats.View;
31+
import io.opencensus.stats.ViewData;
32+
import io.opencensus.tags.TagKey;
33+
import java.util.Arrays;
34+
import java.util.Collections;
2235
import org.junit.Test;
2336
import org.junit.runner.RunWith;
2437
import org.junit.runners.JUnit4;
@@ -31,4 +44,53 @@ public class MutableViewDataTest {
3144
public void testConstants() {
3245
assertThat(MutableViewData.ZERO_TIMESTAMP).isEqualTo(Timestamp.create(0, 0));
3346
}
47+
48+
@Test
49+
public void testTimeRewindsOnCountViewNoThrow() {
50+
// First we set up some buckets THEN we rewind time for giggles.
51+
View tester =
52+
View.create(
53+
View.Name.create("view"),
54+
"Description",
55+
MeasureDouble.create("name", "desc", "us"),
56+
Count.create(),
57+
Collections.singletonList(TagKey.create("KEY")));
58+
Timestamp start = Timestamp.create(10000000, 0);
59+
Timestamp validPointTime = Timestamp.create(10000010, 0);
60+
CurrentState.State state = CurrentState.State.ENABLED;
61+
MutableViewData viewData = MutableViewData.create(tester, start);
62+
// Create a data points to get thrown away.
63+
viewData.record(
64+
TagMapImpl.EMPTY, 1.0, validPointTime, Collections.<String, AttachmentValue>emptyMap());
65+
// Rewind time and look for explosions.
66+
Timestamp thePast = Timestamp.create(0, 0);
67+
ViewData result = viewData.toViewData(thePast, state);
68+
assertThat(result.getAggregationMap()).isEmpty();
69+
}
70+
71+
@Test
72+
public void testTimeRewindsOnDistributionViewNoThrow() {
73+
// First we set up some buckets THEN we rewind time for giggles.
74+
Aggregation latencyDistribution =
75+
Distribution.create(
76+
BucketBoundaries.create(Arrays.asList(0.0, 25.0, 100.0, 200.0, 400.0, 800.0, 10000.0)));
77+
View tester =
78+
View.create(
79+
View.Name.create("view"),
80+
"Description",
81+
MeasureDouble.create("name", "desc", "us"),
82+
latencyDistribution,
83+
Collections.singletonList(TagKey.create("KEY")));
84+
Timestamp start = Timestamp.create(10000000, 0);
85+
Timestamp validPointTime = Timestamp.create(10000010, 0);
86+
CurrentState.State state = CurrentState.State.ENABLED;
87+
MutableViewData viewData = MutableViewData.create(tester, start);
88+
// Create a data points to get thrown away.
89+
viewData.record(
90+
TagMapImpl.EMPTY, 1.0, validPointTime, Collections.<String, AttachmentValue>emptyMap());
91+
// Rewind time and look for explosions.
92+
Timestamp thePast = Timestamp.create(0, 0);
93+
ViewData result = viewData.toViewData(thePast, state);
94+
assertThat(result.getAggregationMap()).isEmpty();
95+
}
3496
}

0 commit comments

Comments
 (0)