honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix problem with TimeIntervalRounding on DST end 

Due to an error in our current TimeIntervalRounding, two dates can
round to the same key, even when they are 1h apart when using
short interval roundings (e.g. 20m) and a time zone with DST change.

Here is an example for the CET time zone:

On 25 October 2015, 03:00:00 clocks are turned backward 1 hour to
02:00:00 local standard time. The dates
"2015-10-25T02:15:00+02:00" (1445732100000) (before DST end) and
"2015-10-25T02:15:00+01:00" (1445735700000) (after DST end)
are thus 1h apart, but currently they round to the same value
"2015-10-25T02:00:00.000+01:00" (1445734800000).

This violates an important invariant of rounding, namely that the
rounded value must be less or equal to the value that is rounded.
It also leads to wrong histogram bucket counts because documents in
[02:00:00+02:00, 02:20:00+02:00) go to the same bucket as documents
from [02:00:00+01:00, 02:20:00+01:00).

The problem happens because in TimeIntervalRounding#roundKey() we
need to perform the rounding operation in local time, but on
converting back to UTC we don't honor the original values time zone
offset. This fix changes that and adds tests both for DST start and
DST end as well as a test that demonstrates what happens to bucket
sizes when the dst change is not evently divisibly by the interval.
This commit is contained in:
Christoph Büscher 2016-05-31 21:42:45 +02:00
parent 712c77264d
commit a2372778dd
2 changed files with 53 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java
View File

@ -216,10 +216,9 @@ public abstract class TimeZoneRounding extends Rounding {
@Override
public long roundKey(long utcMillis) {
long timeLocal = utcMillis;
timeLocal = timeZone.convertUTCToLocal(utcMillis);
long timeLocal = timeZone.convertUTCToLocal(utcMillis);
long rounded = Rounding.Interval.roundValue(Rounding.Interval.roundKey(timeLocal, interval), interval);
return timeZone.convertLocalToUTC(rounded, false);
return timeZone.convertLocalToUTC(rounded, false, utcMillis);
}
@Override

51
core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java
View File

@ -258,6 +258,57 @@ public class TimeZoneRoundingTests extends ESTestCase {
}
}
/**
* test DST end with interval rounding
* CET: 25 October 2015, 03:00:00 clocks were turned backward 1 hour to 25 October 2015, 02:00:00 local standard time
*/
public void testTimeIntervalCET_DST_End() {
long interval = TimeUnit.MINUTES.toMillis(20);
TimeZoneRounding rounding = new TimeIntervalRounding(interval, DateTimeZone.forID("CET"));
assertThat(rounding.round(time("2015-10-25T01:55:00+02:00")), equalTo(time("2015-10-25T01:40:00+02:00")));
assertThat(rounding.round(time("2015-10-25T02:15:00+02:00")), equalTo(time("2015-10-25T02:00:00+02:00")));
assertThat(rounding.round(time("2015-10-25T02:35:00+02:00")), equalTo(time("2015-10-25T02:20:00+02:00")));
assertThat(rounding.round(time("2015-10-25T02:55:00+02:00")), equalTo(time("2015-10-25T02:40:00+02:00")));
// after DST shift
assertThat(rounding.round(time("2015-10-25T02:15:00+01:00")), equalTo(time("2015-10-25T02:00:00+01:00")));
assertThat(rounding.round(time("2015-10-25T02:35:00+01:00")), equalTo(time("2015-10-25T02:20:00+01:00")));
assertThat(rounding.round(time("2015-10-25T02:55:00+01:00")), equalTo(time("2015-10-25T02:40:00+01:00")));
assertThat(rounding.round(time("2015-10-25T03:15:00+01:00")), equalTo(time("2015-10-25T03:00:00+01:00")));
}
/**
* test DST start with interval rounding
* CET: 27 March 2016, 02:00:00 clocks were turned forward 1 hour to 27 March 2016, 03:00:00 local daylight time
*/
public void testTimeIntervalCET_DST_Start() {
long interval = TimeUnit.MINUTES.toMillis(20);
TimeZoneRounding rounding = new TimeIntervalRounding(interval, DateTimeZone.forID("CET"));
// test DST start
assertThat(rounding.round(time("2016-03-27T01:55:00+01:00")), equalTo(time("2016-03-27T01:40:00+01:00")));
assertThat(rounding.round(time("2016-03-27T02:00:00+01:00")), equalTo(time("2016-03-27T03:00:00+02:00")));
assertThat(rounding.round(time("2016-03-27T03:15:00+02:00")), equalTo(time("2016-03-27T03:00:00+02:00")));
assertThat(rounding.round(time("2016-03-27T03:35:00+02:00")), equalTo(time("2016-03-27T03:20:00+02:00")));
}
/**
* test DST start with offset not fitting interval, e.g. Asia/Kathmandu
* adding 15min on 1986-01-01T00:00:00 the interval from
* 1986-01-01T00:15:00+05:45 to 1986-01-01T00:20:00+05:45 to only be 5min
* long
*/
public void testTimeInterval_Kathmandu_DST_Start() {
long interval = TimeUnit.MINUTES.toMillis(20);
TimeZoneRounding rounding = new TimeIntervalRounding(interval, DateTimeZone.forID("Asia/Kathmandu"));
assertThat(rounding.round(time("1985-12-31T23:55:00+05:30")), equalTo(time("1985-12-31T23:40:00+05:30")));
assertThat(rounding.round(time("1986-01-01T00:16:00+05:45")), equalTo(time("1986-01-01T00:15:00+05:45")));
assertThat(time("1986-01-01T00:15:00+05:45") - time("1985-12-31T23:40:00+05:30"), equalTo(TimeUnit.MINUTES.toMillis(20)));
assertThat(rounding.round(time("1986-01-01T00:26:00+05:45")), equalTo(time("1986-01-01T00:20:00+05:45")));
assertThat(time("1986-01-01T00:20:00+05:45") - time("1986-01-01T00:15:00+05:45"), equalTo(TimeUnit.MINUTES.toMillis(5)));
assertThat(rounding.round(time("1986-01-01T00:46:00+05:45")), equalTo(time("1986-01-01T00:40:00+05:45")));
assertThat(time("1986-01-01T00:40:00+05:45") - time("1986-01-01T00:20:00+05:45"), equalTo(TimeUnit.MINUTES.toMillis(20)));
}
/**
* randomized test on {@link TimeIntervalRounding} with random interval and time zone offsets
*/