From 7687e30291243995cffeb7d633a93bf798c29f57 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Sat, 22 Aug 2020 13:11:27 -0400 Subject: [PATCH 01/11] Speed up date_histogram by precomputing ranges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A few of us were talking about ways to speed up the `date_histogram` using the index for the timestamp rather than the doc values. To do that we'd have to pre-compute all of the "round down" points in the index. It turns out that *just* precomputing those values speeds up rounding fairly significantly: ``` Benchmark (count) (interval) (range) (zone) Mode Cnt Score Error Units before 10000000 calendar month 2000-10-28 to 2000-10-31 UTC avgt 10 96461080.982 ± 616373.011 ns/op before 10000000 calendar month 2000-10-28 to 2000-10-31 America/New_York avgt 10 130598950.850 ± 1249189.867 ns/op after 10000000 calendar month 2000-10-28 to 2000-10-31 UTC avgt 10 52311775.080 ± 107171.092 ns/op after 10000000 calendar month 2000-10-28 to 2000-10-31 America/New_York avgt 10 54800134.968 ± 373844.796 ns/op ``` That's a 46% speed up when there isn't a time zone and a 58% speed up when there is. This doesn't work for every time zone, specifically those that have two midnights in a single day due to daylight savings time will produce wonky results. So they don't get the optimization. Second, this requires a few expensive computation up front to make the transition array. And if the transition array is too large then we give up and use the original mechanism, throwing away all of the work we did to build the array. This seems appropriate for most usages of `round`, but this change uses it for *all* usages of `round`. That seems ok for now, but it might be worth investigating in a follow up. I ran a macrobenchmark as well which showed an 11% preformance improvement. *BUT* the benchmark wasn't tuned for my desktop so it overwhelmed it and might have produced "funny" results. I think it is pretty clear that this is an improvement, but know the measurement is weird: ``` Benchmark (count) (interval) (range) (zone) Mode Cnt Score Error Units before 10000000 calendar month 2000-10-28 to 2000-10-31 UTC avgt 10 96461080.982 ± 616373.011 ns/op before 10000000 calendar month 2000-10-28 to 2000-10-31 America/New_York avgt 10 g± 1249189.867 ns/op after 10000000 calendar month 2000-10-28 to 2000-10-31 UTC avgt 10 52311775.080 ± 107171.092 ns/op after 10000000 calendar month 2000-10-28 to 2000-10-31 America/New_York avgt 10 54800134.968 ± 373844.796 ns/op Before: | Min Throughput | hourly_agg | 0.11 | ops/s | | Median Throughput | hourly_agg | 0.11 | ops/s | | Max Throughput | hourly_agg | 0.11 | ops/s | | 50th percentile latency | hourly_agg | 650623 | ms | | 90th percentile latency | hourly_agg | 821478 | ms | | 99th percentile latency | hourly_agg | 859780 | ms | | 100th percentile latency | hourly_agg | 864030 | ms | | 50th percentile service time | hourly_agg | 9268.71 | ms | | 90th percentile service time | hourly_agg | 9380 | ms | | 99th percentile service time | hourly_agg | 9626.88 | ms | |100th percentile service time | hourly_agg | 9884.27 | ms | | error rate | hourly_agg | 0 | % | After: | Min Throughput | hourly_agg | 0.12 | ops/s | | Median Throughput | hourly_agg | 0.12 | ops/s | | Max Throughput | hourly_agg | 0.12 | ops/s | | 50th percentile latency | hourly_agg | 519254 | ms | | 90th percentile latency | hourly_agg | 653099 | ms | | 99th percentile latency | hourly_agg | 683276 | ms | | 100th percentile latency | hourly_agg | 686611 | ms | | 50th percentile service time | hourly_agg | 8371.41 | ms | | 90th percentile service time | hourly_agg | 8407.02 | ms | | 99th percentile service time | hourly_agg | 8536.64 | ms | |100th percentile service time | hourly_agg | 8538.54 | ms | | error rate | hourly_agg | 0 | % | ``` --- .../org/elasticsearch/common/Rounding.java | 70 ++++++++++++++++++- .../elasticsearch/common/RoundingTests.java | 11 ++- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index e1a034fb7924c..c925826334cbd 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.common; +import org.apache.lucene.util.ArrayUtil; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.common.LocalTimeOffset.Gap; @@ -44,8 +45,10 @@ import java.time.temporal.TemporalQueries; import java.time.zone.ZoneOffsetTransition; import java.time.zone.ZoneRules; +import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.concurrent.TimeUnit; /** @@ -401,8 +404,22 @@ private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) { } } + /** + * Time zones with two midnights get "funny" non-continuous rounding + * that isn't compatible with the pre-computed array rounding. + */ + private static final Set HAS_TWO_MIDNIGHTS = Set.of("America/Moncton", "America/St_Johns", "Canada/Newfoundland"); + @Override public Prepared prepare(long minUtcMillis, long maxUtcMillis) { + Prepared orig = prepareOffsetRounding(minUtcMillis, maxUtcMillis); + if (unitRoundsToMidnight && HAS_TWO_MIDNIGHTS.contains(timeZone.getId())) { + return orig; + } + return maybeUseArray(orig, minUtcMillis, maxUtcMillis, 128); + } + + private Prepared prepareOffsetRounding(long minUtcMillis, long maxUtcMillis) { long minLookup = minUtcMillis - unit.extraLocalOffsetLookup(); long maxLookup = maxUtcMillis; @@ -421,7 +438,6 @@ public Prepared prepare(long minUtcMillis, long maxUtcMillis) { // Range too long, just use java.time return prepareJavaTime(); } - LocalTimeOffset fixedOffset = lookup.fixedInRange(minLookup, maxLookup); if (fixedOffset != null) { // The time zone is effectively fixed @@ -1015,7 +1031,7 @@ public byte id() { @Override public Prepared prepare(long minUtcMillis, long maxUtcMillis) { - return wrapPreparedRounding(delegate.prepare(minUtcMillis, maxUtcMillis)); + return wrapPreparedRounding(delegate.prepare(minUtcMillis - offset, maxUtcMillis - offset)); } @Override @@ -1085,4 +1101,54 @@ public static Rounding read(StreamInput in) throws IOException { throw new ElasticsearchException("unknown rounding id [" + id + "]"); } } + + /** + * Attempt to build a {@link Prepared} implementation that relies on pre-calcuated + * "round down" points. If there would be more than {@code max} points then return + * the original implementation, otherwise return the new, faster implementation. + */ + static Prepared maybeUseArray(Prepared orig, long minUtcMillis, long maxUtcMillis, int max) { + long[] values = new long[1]; + long rounded = orig.round(minUtcMillis); + int i = 0; + values[i++] = rounded; + while ((rounded = orig.nextRoundingValue(rounded)) <= maxUtcMillis) { + if (i >= max) { + return orig; + } + assert values[i - 1] == orig.round(rounded - 1); + values = ArrayUtil.grow(values, i + 1); + values[i++]= rounded; + } + return new ArrayRounding(values, i, orig); + } + + /** + * Implementation of {@link Prepared} using pre-calculated "round down" points. + */ + private static class ArrayRounding implements Prepared { + private final long[] values; + private int max; + private final Prepared delegate; + + private ArrayRounding(long[] values, int max, Prepared delegate) { + this.values = values; + this.max = max; + this.delegate = delegate; + } + + @Override + public long round(long utcMillis) { + int idx = Arrays.binarySearch(values, 0, max, utcMillis); + if (idx < 0) { + idx = -2 - idx; + } + return values[idx]; + } + + @Override + public long nextRoundingValue(long utcMillis) { + return delegate.nextRoundingValue(utcMillis); + } + } } diff --git a/server/src/test/java/org/elasticsearch/common/RoundingTests.java b/server/src/test/java/org/elasticsearch/common/RoundingTests.java index cc9220af8906e..b683874266b1e 100644 --- a/server/src/test/java/org/elasticsearch/common/RoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/RoundingTests.java @@ -19,6 +19,8 @@ package org.elasticsearch.common; +import com.carrotsearch.randomizedtesting.annotations.Repeat; + import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.time.DateFormatter; @@ -231,7 +233,7 @@ public void testRandomTimeUnitRounding() { Rounding.DateTimeUnit unit = randomFrom(Rounding.DateTimeUnit.values()); ZoneId tz = randomZone(); Rounding rounding = new Rounding.TimeUnitRounding(unit, tz); - long[] bounds = randomDateBounds(); + long[] bounds = randomDateBounds(unit); Rounding.Prepared prepared = rounding.prepare(bounds[0], bounds[1]); // Check that rounding is internally consistent and consistent with nextRoundingValue @@ -894,8 +896,13 @@ private static long randomDate() { return Math.abs(randomLong() % (2 * (long) 10e11)); // 1970-01-01T00:00:00Z - 2033-05-18T05:33:20.000+02:00 } - private static long[] randomDateBounds() { + private static long[] randomDateBounds(Rounding.DateTimeUnit unit) { long b1 = randomDate(); + if (randomBoolean()) { + // Sometimes use a fairly close date + return new long[] {b1, b1 + unit.extraLocalOffsetLookup() * between(1, 40)}; + } + // Otherwise use a totally random date long b2 = randomValueOtherThan(b1, RoundingTests::randomDate); if (b1 < b2) { return new long[] {b1, b2}; From e68463de6718e51ad5f37692b97877f146cda60b Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 24 Aug 2020 09:38:13 -0400 Subject: [PATCH 02/11] iter --- server/src/main/java/org/elasticsearch/common/Rounding.java | 5 ++++- .../test/java/org/elasticsearch/common/RoundingTests.java | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index c925826334cbd..40a7e3d036639 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -1128,7 +1128,7 @@ static Prepared maybeUseArray(Prepared orig, long minUtcMillis, long maxUtcMilli */ private static class ArrayRounding implements Prepared { private final long[] values; - private int max; + private final int max; private final Prepared delegate; private ArrayRounding(long[] values, int max, Prepared delegate) { @@ -1139,7 +1139,10 @@ private ArrayRounding(long[] values, int max, Prepared delegate) { @Override public long round(long utcMillis) { + assert values[0] <= utcMillis : "utcMillis must be after " + values[0]; int idx = Arrays.binarySearch(values, 0, max, utcMillis); + assert idx != -1 : "The insertion point is before the array! This should have tripped the assertion above."; + assert -1 - idx <= values.length : "This insertion point is after the end of the array."; if (idx < 0) { idx = -2 - idx; } diff --git a/server/src/test/java/org/elasticsearch/common/RoundingTests.java b/server/src/test/java/org/elasticsearch/common/RoundingTests.java index b683874266b1e..e25fd8f8a5523 100644 --- a/server/src/test/java/org/elasticsearch/common/RoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/RoundingTests.java @@ -19,8 +19,6 @@ package org.elasticsearch.common; -import com.carrotsearch.randomizedtesting.annotations.Repeat; - import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.time.DateFormatter; From c13740b5d975d2e19a1521f4fdd6d9de754ee9d1 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 24 Aug 2020 10:23:05 -0400 Subject: [PATCH 03/11] Another test --- .../src/test/java/org/elasticsearch/common/RoundingTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/common/RoundingTests.java b/server/src/test/java/org/elasticsearch/common/RoundingTests.java index e25fd8f8a5523..05a992b9d9dff 100644 --- a/server/src/test/java/org/elasticsearch/common/RoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/RoundingTests.java @@ -213,6 +213,9 @@ public void testOffsetRounding() { assertThat(rounding.nextRoundingValue(0), equalTo(oneDay - twoHours)); assertThat(rounding.withoutOffset().round(0), equalTo(0L)); assertThat(rounding.withoutOffset().nextRoundingValue(0), equalTo(oneDay)); + + rounding = Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).timeZone(ZoneId.of("America/New_York")).offset(-twoHours).build(); + assertThat(rounding.round(time("2020-11-01T09:00:00")), equalTo(time("2020-11-01T02:00:00"))); } /** From 02f0fe3b032c89f896118de6150d23ece8e91eab Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 27 Aug 2020 10:10:00 -0400 Subject: [PATCH 04/11] Implement new method --- server/src/main/java/org/elasticsearch/common/Rounding.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index 495a5e72ed7ef..66d552a03e725 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -1254,5 +1254,10 @@ public long round(long utcMillis) { public long nextRoundingValue(long utcMillis) { return delegate.nextRoundingValue(utcMillis); } + + @Override + public double roundingSize(long utcMillis, DateTimeUnit timeUnit) { + return delegate.roundingSize(utcMillis, timeUnit); + } } } From 4fbcfbf0ae707e70536d9796aaa65ac6035f7a6f Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 31 Aug 2020 10:47:06 -0400 Subject: [PATCH 05/11] Skip method for bad times --- .../org/elasticsearch/common/Rounding.java | 21 ++- .../elasticsearch/common/RoundingTests.java | 152 +++++++++++++----- 2 files changed, 127 insertions(+), 46 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index 66d552a03e725..234fa4384570f 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -48,8 +48,8 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.concurrent.TimeUnit; /** @@ -471,15 +471,26 @@ private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) { } /** - * Time zones with two midnights get "funny" non-continuous rounding - * that isn't compatible with the pre-computed array rounding. + * Time zones have a daylight savings time transition after midnight that + * transitions back before midnight break the array-based rounding so + * we don't use it for them during those transitions. Luckily they are + * fairly rare and a while in the past. Note: we can use the array based + * rounding if the transition is at midnight. Just not + * if it is after it. */ - private static final Set HAS_TWO_MIDNIGHTS = Set.of("America/Moncton", "America/St_Johns", "Canada/Newfoundland"); + private static final Map FORWARDS_BACKWRADS_ZONES = Map.ofEntries( + Map.entry("America/Goose_Bay", 1289116800000L), // Stopped transitioning after midnight in 2010 + Map.entry("America/Moncton", 1162108800000L), // Stopped transitioning after midnight in 2006 + Map.entry("America/St_Johns", 1289118600000L), // Stopped transitioning after midnight in 2010 + Map.entry("Canada/Newfoundland", 1289118600000L), // Stopped transitioning after midnight in 2010 + Map.entry("Pacific/Guam", -29347200000L), // Stopped transitioning after midnight in 1969 + Map.entry("Pacific/Saipan", -29347200000L) // Stopped transitioning after midnight in 1969 + ); @Override public Prepared prepare(long minUtcMillis, long maxUtcMillis) { Prepared orig = prepareOffsetRounding(minUtcMillis, maxUtcMillis); - if (unitRoundsToMidnight && HAS_TWO_MIDNIGHTS.contains(timeZone.getId())) { + if (unitRoundsToMidnight && minUtcMillis <= FORWARDS_BACKWRADS_ZONES.getOrDefault(timeZone.getId(), Long.MIN_VALUE)) { return orig; } return maybeUseArray(orig, minUtcMillis, maxUtcMillis, 128); diff --git a/server/src/test/java/org/elasticsearch/common/RoundingTests.java b/server/src/test/java/org/elasticsearch/common/RoundingTests.java index 0b4fb23f89915..841168d51037c 100644 --- a/server/src/test/java/org/elasticsearch/common/RoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/RoundingTests.java @@ -36,6 +36,8 @@ import java.time.format.DateTimeFormatter; import java.time.temporal.TemporalAccessor; import java.time.zone.ZoneOffsetTransition; +import java.time.zone.ZoneOffsetTransitionRule; +import java.time.zone.ZoneRules; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -234,51 +236,55 @@ public void testRandomTimeUnitRounding() { for (int i = 0; i < 1000; ++i) { Rounding.DateTimeUnit unit = randomFrom(Rounding.DateTimeUnit.values()); ZoneId tz = randomZone(); - Rounding rounding = new Rounding.TimeUnitRounding(unit, tz); long[] bounds = randomDateBounds(unit); - Rounding.Prepared prepared = rounding.prepare(bounds[0], bounds[1]); + assertUnitRoundingSameAsJavaUtilTimeImplementation(unit, tz, bounds[0], bounds[1]); + } + } - // Check that rounding is internally consistent and consistent with nextRoundingValue - long date = dateBetween(bounds[0], bounds[1]); - long unitMillis = unit.getField().getBaseUnit().getDuration().toMillis(); - // FIXME this was copy pasted from the other impl and not used. breaks the nasty date actually gets assigned - if (randomBoolean()) { - nastyDate(date, tz, unitMillis); - } - final long roundedDate = prepared.round(date); - final long nextRoundingValue = prepared.nextRoundingValue(roundedDate); - - assertInterval(roundedDate, date, nextRoundingValue, rounding, tz); - - // check correct unit interval width for units smaller than a day, they should be fixed size except for transitions - if (unitMillis <= 86400 * 1000) { - // if the interval defined didn't cross timezone offset transition, it should cover unitMillis width - int offsetRounded = tz.getRules().getOffset(Instant.ofEpochMilli(roundedDate - 1)).getTotalSeconds(); - int offsetNextValue = tz.getRules().getOffset(Instant.ofEpochMilli(nextRoundingValue + 1)).getTotalSeconds(); - if (offsetRounded == offsetNextValue) { - assertThat("unit interval width not as expected for [" + unit + "], [" + tz + "] at " - + Instant.ofEpochMilli(roundedDate), nextRoundingValue - roundedDate, equalTo(unitMillis)); - } + private void assertUnitRoundingSameAsJavaUtilTimeImplementation(Rounding.DateTimeUnit unit, ZoneId tz, long start, long end) { + Rounding rounding = new Rounding.TimeUnitRounding(unit, tz); + Rounding.Prepared prepared = rounding.prepare(start, end); + + // Check that rounding is internally consistent and consistent with nextRoundingValue + long date = dateBetween(start, end); + long unitMillis = unit.getField().getBaseUnit().getDuration().toMillis(); + // FIXME this was copy pasted from the other impl and not used. breaks the nasty date actually gets assigned + if (randomBoolean()) { + nastyDate(date, tz, unitMillis); + } + final long roundedDate = prepared.round(date); + final long nextRoundingValue = prepared.nextRoundingValue(roundedDate); + + assertInterval(roundedDate, date, nextRoundingValue, rounding, tz); + + // check correct unit interval width for units smaller than a day, they should be fixed size except for transitions + if (unitMillis <= 86400 * 1000) { + // if the interval defined didn't cross timezone offset transition, it should cover unitMillis width + int offsetRounded = tz.getRules().getOffset(Instant.ofEpochMilli(roundedDate - 1)).getTotalSeconds(); + int offsetNextValue = tz.getRules().getOffset(Instant.ofEpochMilli(nextRoundingValue + 1)).getTotalSeconds(); + if (offsetRounded == offsetNextValue) { + assertThat("unit interval width not as expected for [" + unit + "], [" + tz + "] at " + + Instant.ofEpochMilli(roundedDate), nextRoundingValue - roundedDate, equalTo(unitMillis)); } + } - // Round a whole bunch of dates and make sure they line up with the known good java time implementation - Rounding.Prepared javaTimeRounding = rounding.prepareJavaTime(); - for (int d = 0; d < 1000; d++) { - date = dateBetween(bounds[0], bounds[1]); - long javaRounded = javaTimeRounding.round(date); - long esRounded = prepared.round(date); - if (javaRounded != esRounded) { - fail("Expected [" + rounding + "] to round [" + Instant.ofEpochMilli(date) + "] to [" - + Instant.ofEpochMilli(javaRounded) + "] but instead rounded to [" + Instant.ofEpochMilli(esRounded) + "]"); - } - long javaNextRoundingValue = javaTimeRounding.nextRoundingValue(date); - long esNextRoundingValue = prepared.nextRoundingValue(date); - if (javaNextRoundingValue != esNextRoundingValue) { - fail("Expected [" + rounding + "] to round [" + Instant.ofEpochMilli(date) + "] to [" - + Instant.ofEpochMilli(esRounded) + "] and nextRoundingValue to be [" - + Instant.ofEpochMilli(javaNextRoundingValue) + "] but instead was to [" - + Instant.ofEpochMilli(esNextRoundingValue) + "]"); - } + // Round a whole bunch of dates and make sure they line up with the known good java time implementation + Rounding.Prepared javaTimeRounding = rounding.prepareJavaTime(); + for (int d = 0; d < 1000; d++) { + date = dateBetween(start, end); + long javaRounded = javaTimeRounding.round(date); + long esRounded = prepared.round(date); + if (javaRounded != esRounded) { + fail("Expected [" + rounding + "] to round [" + Instant.ofEpochMilli(date) + "] to [" + + Instant.ofEpochMilli(javaRounded) + "] but instead rounded to [" + Instant.ofEpochMilli(esRounded) + "]"); + } + long javaNextRoundingValue = javaTimeRounding.nextRoundingValue(date); + long esNextRoundingValue = prepared.nextRoundingValue(date); + if (javaNextRoundingValue != esNextRoundingValue) { + fail("Expected [" + rounding + "] to round [" + Instant.ofEpochMilli(date) + "] to [" + + Instant.ofEpochMilli(esRounded) + "] and nextRoundingValue to be [" + + Instant.ofEpochMilli(javaNextRoundingValue) + "] but instead was to [" + + Instant.ofEpochMilli(esNextRoundingValue) + "]"); } } } @@ -718,6 +724,70 @@ public void testDST_America_St_Johns() { } } + /** + * Tests for DST transitions that cause the rounding to jump "backwards" because they round + * from one back to the previous day. Usually these rounding start before + */ + public void testForwardsBackwardsTimeZones() { + for (String zoneId : JAVA_ZONE_IDS) { + ZoneId tz = ZoneId.of(zoneId); + ZoneRules rules = tz.getRules(); + for (ZoneOffsetTransition transition : rules.getTransitions()) { + checkForForwardsBackwardsTransition(tz, transition); + } + int firstYear; + if (rules.getTransitions().isEmpty()) { + // Pick an arbitrary year to start the range + firstYear = 1999; + } else { + ZoneOffsetTransition lastTransition = rules.getTransitions().get(rules.getTransitions().size() - 1); + firstYear = lastTransition.getDateTimeAfter().getYear() + 1; + } + // Pick an arbitrary year to end the range too + int lastYear = 2050; + int year = randomFrom(firstYear, lastYear); + for (ZoneOffsetTransitionRule transitionRule : rules.getTransitionRules()) { + ZoneOffsetTransition transition = transitionRule.createTransition(year); + checkForForwardsBackwardsTransition(tz, transition); + } + } + } + + private void checkForForwardsBackwardsTransition(ZoneId tz, ZoneOffsetTransition transition) { + if (transition.getDateTimeBefore().getYear() < 1950) { + // We don't support transitions far in the past at all + return; + } + if (false == transition.isOverlap()) { + // Only overlaps cause the array rounding to have trouble + return; + } + if (transition.getDateTimeBefore().getDayOfMonth() == transition.getDateTimeAfter().getDayOfMonth()) { + // Only when the rounding changes the day + return; + } + if (transition.getDateTimeBefore().getMinute() == 0) { + // But roundings that change *at* midnight are safe because they don't "jump" to the next day. + return; + } + logger.info( + "{} from {}{} to {}{}", + tz, + transition.getDateTimeBefore(), + transition.getOffsetBefore(), + transition.getDateTimeAfter(), + transition.getOffsetAfter() + ); + long millisSinceEpoch = TimeUnit.SECONDS.toMillis(transition.toEpochSecond()); + long twoHours = TimeUnit.HOURS.toMillis(2); + assertUnitRoundingSameAsJavaUtilTimeImplementation( + Rounding.DateTimeUnit.DAY_OF_MONTH, + tz, + millisSinceEpoch - twoHours, + millisSinceEpoch + twoHours + ); + } + /** * tests for dst transition with overlaps and day roundings. */ From 178a1586527a2f208189fc11a904c1f27c8ee3de Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 14 Sep 2020 09:36:52 -0400 Subject: [PATCH 06/11] comments --- .../org/elasticsearch/common/Rounding.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index 234fa4384570f..a484f69c99a84 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -471,12 +471,15 @@ private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) { } /** - * Time zones have a daylight savings time transition after midnight that - * transitions back before midnight break the array-based rounding so - * we don't use it for them during those transitions. Luckily they are - * fairly rare and a while in the past. Note: we can use the array based - * rounding if the transition is at midnight. Just not - * if it is after it. + * Time zones that have a daylight savings time transition after midnight + * that transitions back before midnight which breaks the array-based rounding. + * This is a map from the id of the zone to the last time when such a transition + * occurred in millis since epoch. We do not use the array based rounding when any + * times come before this transition. + *

+ * Note: we can use the array based rounding if the transition is + * at midnight and transitions before it. Just not if it is + * after midnight and jumps before it. */ private static final Map FORWARDS_BACKWRADS_ZONES = Map.ofEntries( Map.entry("America/Goose_Bay", 1289116800000L), // Stopped transitioning after midnight in 2010 @@ -1228,6 +1231,11 @@ static Prepared maybeUseArray(Prepared orig, long minUtcMillis, long maxUtcMilli if (i >= max) { return orig; } + /* + * We expect a time in the last transition (rounded - 1) to round + * to the last value we calculated. If it doesn't then we're + * probably doing something wrong here.... + */ assert values[i - 1] == orig.round(rounded - 1); values = ArrayUtil.grow(values, i + 1); values[i++]= rounded; From f201a25ba7be7f2dace68730a1e2abb8458d79c3 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 14 Sep 2020 12:00:21 -0400 Subject: [PATCH 07/11] Detect! --- .../elasticsearch/common/LocalTimeOffset.java | 79 ++++++++++++++- .../org/elasticsearch/common/Rounding.java | 95 ++++++++----------- .../common/LocalTimeOffsetTests.java | 27 +++++- .../elasticsearch/common/RoundingTests.java | 5 + 4 files changed, 144 insertions(+), 62 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java b/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java index 1365cb15056e5..5de963a94c574 100644 --- a/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java +++ b/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java @@ -174,6 +174,12 @@ public interface Strategy { */ protected abstract LocalTimeOffset offsetContaining(long utcMillis); + /** + * Does this transition or any previous transitions move back to the + * previous day? See {@link Lookup#anyMoveBackToPreviousDay()} for rules. + */ + protected abstract boolean anyMoveBackToPreviousDay(); + @Override public String toString() { return toString(millis); @@ -195,6 +201,15 @@ public abstract static class Lookup { */ public abstract LocalTimeOffset fixedInRange(long minUtcMillis, long maxUtcMillis); + /** + * Do any of the transitions move back to the previous day? + *

+ * Note: If an overlap occurs at, say, 1 am and jumps back to + * exactly midnight then it doesn't count because + * midnight is still counted as being in the "next" day. + */ + public abstract boolean anyMoveBackToPreviousDay(); + /** * The number of offsets in the lookup. Package private for testing. */ @@ -225,6 +240,11 @@ protected LocalTimeOffset offsetContaining(long utcMillis) { return this; } + @Override + protected boolean anyMoveBackToPreviousDay() { + return false; + } + @Override protected String toString(long millis) { return Long.toString(millis); @@ -298,6 +318,11 @@ public long firstMissingLocalTime() { return firstMissingLocalTime; } + @Override + protected boolean anyMoveBackToPreviousDay() { + return previous().anyMoveBackToPreviousDay(); + } + @Override protected String toString(long millis) { return "Gap of " + millis + "@" + Instant.ofEpochMilli(startUtcMillis()); @@ -307,13 +332,21 @@ protected String toString(long millis) { public static class Overlap extends Transition { private final long firstOverlappingLocalTime; private final long firstNonOverlappingLocalTime; - - private Overlap(long millis, LocalTimeOffset previous, long startUtcMillis, - long firstOverlappingLocalTime, long firstNonOverlappingLocalTime) { + private final boolean movesBackToPreviousDay; + + private Overlap( + long millis, + LocalTimeOffset previous, + long startUtcMillis, + long firstOverlappingLocalTime, + long firstNonOverlappingLocalTime, + boolean movesBackToPreviousDay + ) { super(millis, previous, startUtcMillis); this.firstOverlappingLocalTime = firstOverlappingLocalTime; this.firstNonOverlappingLocalTime = firstNonOverlappingLocalTime; assert firstOverlappingLocalTime < firstNonOverlappingLocalTime; + this.movesBackToPreviousDay = movesBackToPreviousDay; } @Override @@ -341,6 +374,11 @@ public long firstOverlappingLocalTime() { return firstOverlappingLocalTime; } + @Override + protected boolean anyMoveBackToPreviousDay() { + return movesBackToPreviousDay || previous().anyMoveBackToPreviousDay(); + } + @Override protected String toString(long millis) { return "Overlap of " + millis + "@" + Instant.ofEpochMilli(startUtcMillis()); @@ -375,6 +413,11 @@ int size() { public String toString() { return String.format(Locale.ROOT, "FixedLookup[for %s at %s]", zone, fixed); } + + @Override + public boolean anyMoveBackToPreviousDay() { + return false; + } } /** @@ -406,6 +449,11 @@ public LocalTimeOffset innerLookup(long utcMillis) { int size() { return size; } + + @Override + public boolean anyMoveBackToPreviousDay() { + return lastOffset.anyMoveBackToPreviousDay(); + } } /** @@ -453,6 +501,11 @@ int size() { return offsets.length; } + @Override + public boolean anyMoveBackToPreviousDay() { + return offsets[offsets.length - 1].anyMoveBackToPreviousDay(); + } + @Override public String toString() { return String.format(Locale.ROOT, "TransitionArrayLookup[for %s between %s and %s]", @@ -505,7 +558,25 @@ protected static Transition buildTransition(ZoneOffsetTransition transition, Loc } long firstOverlappingLocalTime = utcStart + offsetAfterMillis; long firstNonOverlappingLocalTime = utcStart + offsetBeforeMillis; - return new Overlap(offsetAfterMillis, previous, utcStart, firstOverlappingLocalTime, firstNonOverlappingLocalTime); + return new Overlap( + offsetAfterMillis, + previous, + utcStart, + firstOverlappingLocalTime, + firstNonOverlappingLocalTime, + movesBackToPreviousDay(transition) + ); + } + + private static boolean movesBackToPreviousDay(ZoneOffsetTransition transition) { + if (transition.getDateTimeBefore().getDayOfMonth() == transition.getDateTimeAfter().getDayOfMonth()) { + return false; + } + if (transition.getDateTimeBefore().getMinute() == 0) { + // If we change *at* midnight this is ok. + return false; + } + return true; } } diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index a484f69c99a84..1a3ef2e40adce 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -48,7 +48,6 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; @@ -408,6 +407,34 @@ public Rounding build() { } } + private abstract class PreparedRounding implements Prepared { + /** + * Attempt to build a {@link Prepared} implementation that relies on pre-calcuated + * "round down" points. If there would be more than {@code max} points then return + * the original implementation, otherwise return the new, faster implementation. + */ + protected Prepared maybeUseArray(long minUtcMillis, long maxUtcMillis, int max) { + long[] values = new long[1]; + long rounded = round(minUtcMillis); + int i = 0; + values[i++] = rounded; + while ((rounded = nextRoundingValue(rounded)) <= maxUtcMillis) { + if (i >= max) { + return this; + } + /* + * We expect a time in the last transition (rounded - 1) to round + * to the last value we calculated. If it doesn't then we're + * probably doing something wrong here.... + */ + assert values[i - 1] == round(rounded - 1); + values = ArrayUtil.grow(values, i + 1); + values[i++]= rounded; + } + return new ArrayRounding(values, i, this); + } + } + static class TimeUnitRounding extends Rounding { static final byte ID = 1; @@ -470,36 +497,12 @@ private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) { } } - /** - * Time zones that have a daylight savings time transition after midnight - * that transitions back before midnight which breaks the array-based rounding. - * This is a map from the id of the zone to the last time when such a transition - * occurred in millis since epoch. We do not use the array based rounding when any - * times come before this transition. - *

- * Note: we can use the array based rounding if the transition is - * at midnight and transitions before it. Just not if it is - * after midnight and jumps before it. - */ - private static final Map FORWARDS_BACKWRADS_ZONES = Map.ofEntries( - Map.entry("America/Goose_Bay", 1289116800000L), // Stopped transitioning after midnight in 2010 - Map.entry("America/Moncton", 1162108800000L), // Stopped transitioning after midnight in 2006 - Map.entry("America/St_Johns", 1289118600000L), // Stopped transitioning after midnight in 2010 - Map.entry("Canada/Newfoundland", 1289118600000L), // Stopped transitioning after midnight in 2010 - Map.entry("Pacific/Guam", -29347200000L), // Stopped transitioning after midnight in 1969 - Map.entry("Pacific/Saipan", -29347200000L) // Stopped transitioning after midnight in 1969 - ); - @Override public Prepared prepare(long minUtcMillis, long maxUtcMillis) { - Prepared orig = prepareOffsetRounding(minUtcMillis, maxUtcMillis); - if (unitRoundsToMidnight && minUtcMillis <= FORWARDS_BACKWRADS_ZONES.getOrDefault(timeZone.getId(), Long.MIN_VALUE)) { - return orig; - } - return maybeUseArray(orig, minUtcMillis, maxUtcMillis, 128); + return prepareOffsetOrJavaTimeRounding(minUtcMillis, maxUtcMillis).maybeUseArray(minUtcMillis, maxUtcMillis, 128); } - private Prepared prepareOffsetRounding(long minUtcMillis, long maxUtcMillis) { + private TimeUnitPreparedRounding prepareOffsetOrJavaTimeRounding(long minUtcMillis, long maxUtcMillis) { long minLookup = minUtcMillis - unit.extraLocalOffsetLookup(); long maxLookup = maxUtcMillis; @@ -546,7 +549,7 @@ public Prepared prepareForUnknown() { } @Override - Prepared prepareJavaTime() { + TimeUnitPreparedRounding prepareJavaTime() { if (unitRoundsToMidnight) { return new JavaTimeToMidnightRounding(); } @@ -585,7 +588,7 @@ public String toString() { return "Rounding[" + unit + " in " + timeZone + "]"; } - private abstract class TimeUnitPreparedRounding implements Prepared { + private abstract class TimeUnitPreparedRounding extends PreparedRounding { @Override public double roundingSize(long utcMillis, DateTimeUnit timeUnit) { if (timeUnit.isMillisBased == unit.isMillisBased) { @@ -679,6 +682,14 @@ public long inOverlap(long localMillis, Overlap overlap) { public long beforeOverlap(long localMillis, Overlap overlap) { return overlap.previous().localToUtc(localMillis, this); } + + @Override + protected Prepared maybeUseArray(long minUtcMillis, long maxUtcMillis, int max) { + if (lookup.anyMoveBackToPreviousDay()) { + return this; + } + return super.maybeUseArray(minUtcMillis, maxUtcMillis, max); + } } private class NotToMidnightRounding extends AbstractNotToMidnightRounding implements LocalTimeOffset.Strategy { @@ -1217,32 +1228,6 @@ public static Rounding read(StreamInput in) throws IOException { } } - /** - * Attempt to build a {@link Prepared} implementation that relies on pre-calcuated - * "round down" points. If there would be more than {@code max} points then return - * the original implementation, otherwise return the new, faster implementation. - */ - static Prepared maybeUseArray(Prepared orig, long minUtcMillis, long maxUtcMillis, int max) { - long[] values = new long[1]; - long rounded = orig.round(minUtcMillis); - int i = 0; - values[i++] = rounded; - while ((rounded = orig.nextRoundingValue(rounded)) <= maxUtcMillis) { - if (i >= max) { - return orig; - } - /* - * We expect a time in the last transition (rounded - 1) to round - * to the last value we calculated. If it doesn't then we're - * probably doing something wrong here.... - */ - assert values[i - 1] == orig.round(rounded - 1); - values = ArrayUtil.grow(values, i + 1); - values[i++]= rounded; - } - return new ArrayRounding(values, i, orig); - } - /** * Implementation of {@link Prepared} using pre-calculated "round down" points. */ diff --git a/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java b/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java index 2b762500f5267..7ed5bb465ec01 100644 --- a/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java +++ b/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java @@ -51,20 +51,22 @@ public void testNotFixed() { } public void testUtc() { - assertFixOffset(ZoneId.of("UTC"), 0); + assertFixedOffset(ZoneId.of("UTC"), 0); } public void testFixedOffset() { ZoneOffset zone = ZoneOffset.ofTotalSeconds(between((int) -TimeUnit.HOURS.toSeconds(18), (int) TimeUnit.HOURS.toSeconds(18))); - assertFixOffset(zone, zone.getTotalSeconds() * 1000); + assertFixedOffset(zone, zone.getTotalSeconds() * 1000); } - private void assertFixOffset(ZoneId zone, long offsetMillis) { + private void assertFixedOffset(ZoneId zone, long offsetMillis) { LocalTimeOffset fixed = LocalTimeOffset.fixedOffset(zone); assertThat(fixed, notNullValue()); LocalTimeOffset.Lookup lookup = LocalTimeOffset.lookup(zone, Long.MIN_VALUE, Long.MAX_VALUE); assertThat(lookup.size(), equalTo(1)); + assertThat(lookup.anyMoveBackToPreviousDay(), equalTo(false)); + long min = randomLong(); long max = randomValueOtherThan(min, ESTestCase::randomLong); if (min > max) { @@ -72,8 +74,10 @@ private void assertFixOffset(ZoneId zone, long offsetMillis) { min = max; max = s; } + LocalTimeOffset fixedInRange = lookup.fixedInRange(min, max); assertThat(fixedInRange, notNullValue()); + assertThat(fixedInRange.anyMoveBackToPreviousDay(), equalTo(false)); assertRoundingAtOffset(randomBoolean() ? fixed : fixedInRange, randomLong(), offsetMillis); } @@ -118,6 +122,7 @@ private void assertTransitions(ZoneId zone, long min, long max, long between, lo assertRoundingAtOffset(lookup.lookup(max), max, minMaxOffset); assertThat(lookup.fixedInRange(min, max), nullValue()); assertThat(lookup.fixedInRange(min, sameOffsetAsMin), sameInstance(lookup.lookup(min))); + assertThat(lookup.anyMoveBackToPreviousDay(), equalTo(false)); // None of the examples we use this method with move back } // Some sanity checks for when you pas a single time. We don't expect to do this much but it shouldn't be totally borked. @@ -241,6 +246,22 @@ public void testGap() { assertThat(gapOffset.localToUtc(localSkippedTime, useValueForGap(gapValue)), equalTo(gapValue)); } + public void testKnownMovesBackToPreviousDay() { + assertKnownMovesBacktoPreviousDay("America/Goose_Bay", "2010-11-07T03:01:00"); + assertKnownMovesBacktoPreviousDay("America/Moncton", "2006-10-29T03:01:00"); + assertKnownMovesBacktoPreviousDay("America/Moncton", "2005-10-29T03:01:00"); + assertKnownMovesBacktoPreviousDay("America/St_Johns", "2010-11-07T02:31:00"); + assertKnownMovesBacktoPreviousDay("Canada/Newfoundland", "2010-11-07T02:31:00"); + assertKnownMovesBacktoPreviousDay("Pacific/Guam", "1969-01-25T13:01:00"); + assertKnownMovesBacktoPreviousDay("Pacific/Saipan", "1969-01-25T13:01:00"); + assertKnownMovesBacktoPreviousDay("Europe/Paris", "1911-03-1T00:01:00"); + } + + private void assertKnownMovesBacktoPreviousDay(String zone, String time) { + long utc = utcTime(time); + assertTrue(LocalTimeOffset.lookup(ZoneId.of(zone), utc, utc + 1).anyMoveBackToPreviousDay()); + } + private static long utcTime(String time) { return DateFormatter.forPattern("date_optional_time").parseMillis(time); } diff --git a/server/src/test/java/org/elasticsearch/common/RoundingTests.java b/server/src/test/java/org/elasticsearch/common/RoundingTests.java index 841168d51037c..e0d464c875f15 100644 --- a/server/src/test/java/org/elasticsearch/common/RoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/RoundingTests.java @@ -1030,6 +1030,11 @@ private static boolean isTimeWithWellDefinedRounding(ZoneId tz, long t) { return t <= time("2010-03-03T00:00:00Z") || t >= time("2010-03-07T00:00:00Z"); } + if (tz.getId().equals("Pacific/Guam") || tz.getId().equals("Pacific/Saipan")) { + // Clocks went back at 00:01 in 1969, causing overlapping days. + return t <= time("1969-01-25T00:00:00Z") + || t >= time("1969-01-26T00:00:00Z"); + } return true; } From 0e937293300f2c2796e1cf486350f6859d110464 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 14 Sep 2020 13:50:12 -0400 Subject: [PATCH 08/11] Test update --- server/src/main/java/org/elasticsearch/common/Rounding.java | 6 ++++++ .../java/org/elasticsearch/common/LocalTimeOffsetTests.java | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index 1a3ef2e40adce..8d237a565c29b 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -749,6 +749,12 @@ public long nextRoundingValue(long utcMillis) { return firstTimeOnDay(localMidnight); } + @Override + protected Prepared maybeUseArray(long minUtcMillis, long maxUtcMillis, int max) { + // We don't have the right information needed to know if this is safe for this time zone so we always use java rounding + return this; + } + private long firstTimeOnDay(LocalDateTime localMidnight) { assert localMidnight.toLocalTime().equals(LocalTime.of(0, 0, 0)) : "firstTimeOnDay should only be called at midnight"; diff --git a/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java b/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java index 7ed5bb465ec01..1d41fdf5d008b 100644 --- a/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java +++ b/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java @@ -259,7 +259,10 @@ public void testKnownMovesBackToPreviousDay() { private void assertKnownMovesBacktoPreviousDay(String zone, String time) { long utc = utcTime(time); - assertTrue(LocalTimeOffset.lookup(ZoneId.of(zone), utc, utc + 1).anyMoveBackToPreviousDay()); + assertTrue( + zone + " just after " + time + " should move back", + LocalTimeOffset.lookup(ZoneId.of(zone), utc, utc + 1).anyMoveBackToPreviousDay() + ); } private static long utcTime(String time) { From 35ef1c4e6f8757dcde5f47ba19678d9a9cd9a493 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 14 Sep 2020 14:07:00 -0400 Subject: [PATCH 09/11] Comment on number --- server/src/main/java/org/elasticsearch/common/Rounding.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index 8d237a565c29b..713e8feec2b06 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -499,6 +499,11 @@ private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) { @Override public Prepared prepare(long minUtcMillis, long maxUtcMillis) { + /* + * 128 is a power of two that isn't huge. We might be able to do + * better if the limit was based on the actual type of prepared + * rounding but this'll do for now. + */ return prepareOffsetOrJavaTimeRounding(minUtcMillis, maxUtcMillis).maybeUseArray(minUtcMillis, maxUtcMillis, 128); } From f1e286ebba0ce02bb37d3ee4ef8696852e6aec7b Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 15 Sep 2020 13:57:27 -0400 Subject: [PATCH 10/11] Only 12+ --- .../org/elasticsearch/common/LocalTimeOffsetTests.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java b/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java index 1d41fdf5d008b..1455c3c0db062 100644 --- a/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java +++ b/server/src/test/java/org/elasticsearch/common/LocalTimeOffsetTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.common; +import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.common.LocalTimeOffset.Gap; import org.elasticsearch.common.LocalTimeOffset.Overlap; import org.elasticsearch.common.time.DateFormatter; @@ -252,9 +253,12 @@ public void testKnownMovesBackToPreviousDay() { assertKnownMovesBacktoPreviousDay("America/Moncton", "2005-10-29T03:01:00"); assertKnownMovesBacktoPreviousDay("America/St_Johns", "2010-11-07T02:31:00"); assertKnownMovesBacktoPreviousDay("Canada/Newfoundland", "2010-11-07T02:31:00"); - assertKnownMovesBacktoPreviousDay("Pacific/Guam", "1969-01-25T13:01:00"); - assertKnownMovesBacktoPreviousDay("Pacific/Saipan", "1969-01-25T13:01:00"); assertKnownMovesBacktoPreviousDay("Europe/Paris", "1911-03-1T00:01:00"); + if (JavaVersion.current().compareTo(JavaVersion.parse("11")) > 0) { + // Added in java 12 + assertKnownMovesBacktoPreviousDay("Pacific/Guam", "1969-01-25T13:01:00"); + assertKnownMovesBacktoPreviousDay("Pacific/Saipan", "1969-01-25T13:01:00"); + } } private void assertKnownMovesBacktoPreviousDay(String zone, String time) { From 2281d351f748d7b5030deeb04355f32616c444ed Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 24 Sep 2020 08:42:36 -0400 Subject: [PATCH 11/11] Nano of day! --- .../main/java/org/elasticsearch/common/LocalTimeOffset.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java b/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java index 5de963a94c574..0f5f23c6ef6ad 100644 --- a/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java +++ b/server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java @@ -22,6 +22,7 @@ import java.time.Instant; import java.time.LocalDate; import java.time.ZoneId; +import java.time.temporal.ChronoField; import java.time.zone.ZoneOffsetTransition; import java.time.zone.ZoneOffsetTransitionRule; import java.time.zone.ZoneRules; @@ -572,7 +573,7 @@ private static boolean movesBackToPreviousDay(ZoneOffsetTransition transition) { if (transition.getDateTimeBefore().getDayOfMonth() == transition.getDateTimeAfter().getDayOfMonth()) { return false; } - if (transition.getDateTimeBefore().getMinute() == 0) { + if (transition.getDateTimeBefore().getLong(ChronoField.NANO_OF_DAY) == 0L) { // If we change *at* midnight this is ok. return false; }