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

Make Rounding.nextRoundingValue consistent (backport #62983) (#63242) 

"interval" style roundings were implementing `nextRoundingValue` in a
fairly inconsistent way - it'd produce a value, but sometimes that
value would be the same as the previous rounding value. This makes it
consistently the next value that `rounding` would make.
This commit is contained in:
Nik Everett 2020-10-05 10:38:20 -04:00 committed by GitHub
parent 1b32daf37b
commit 461475f9e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 108 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
server/src/main/java/org/elasticsearch/common/Rounding.java
View File

@ -18,6 +18,8 @@
*/
package org.elasticsearch.common;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.ArrayUtil;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
@ -37,7 +39,6 @@ import java.time.LocalTime;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoField;
import java.time.temporal.ChronoUnit;
import java.time.temporal.IsoFields;
@ -63,6 +64,8 @@ import java.util.concurrent.TimeUnit;
* a comedy gold mine. If you like time zones. Or hate them.
*/
public abstract class Rounding implements Writeable {
private static final Logger logger = LogManager.getLogger(Rounding.class);
public enum DateTimeUnit {
WEEK_OF_WEEKYEAR(
(byte) 1,
@ -1130,12 +1133,65 @@ public abstract class Rounding implements Writeable {
}
@Override
public long nextRoundingValue(long time) {
int offsetSeconds = timeZone.getRules().getOffset(Instant.ofEpochMilli(time)).getTotalSeconds();
long millis = time + interval + offsetSeconds * 1000;
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC)
.withZoneSameLocal(timeZone)
.toInstant().toEpochMilli();
public long nextRoundingValue(long utcMillis) {
/*
* Ok. I'm not proud of this, but it gets the job done. So here is the deal:
* its super important that nextRoundingValue be *exactly* the next rounding
* value. And I can't come up with a nice way to use the java time API to figure
* it out. Thus, we treat "round" like a black box here and run a kind of whacky
* binary search, newton's method hybrid. We don't have a "slope" so we can't do
* a "real" newton's method, so we just sort of cut the diff in half. As janky
* as it looks, it tends to get the job done in under four iterations. Frankly,
* `round(round(utcMillis) + interval)` is usually a good guess so we mostly get
* it in a single iteration. But daylight savings time and other janky stuff can
* make it less likely.
*/
long prevRound = round(utcMillis);
long increment = interval;
long from = prevRound;
int iterations = 0;
while (++iterations < 100) {
from += increment;
long rounded = round(from);
boolean highEnough = rounded > prevRound;
if (false == highEnough) {
if (increment < 0) {
increment = -increment / 2;
}
continue;
}
long roundedRoundedDown = round(rounded - 1);
boolean tooHigh = roundedRoundedDown > prevRound;
if (tooHigh) {
if (increment > 0) {
increment = -increment / 2;
}
continue;
}
assert highEnough && (false == tooHigh);
assert roundedRoundedDown == prevRound;
if (iterations > 3 && logger.isDebugEnabled()) {
logger.debug("Iterated {} time for {} using {}", iterations, utcMillis, TimeIntervalRounding.this.toString());
}
return rounded;
}
/*
* After 100 iterations we still couldn't settle on something! Crazy!
* The most I've seen in tests is 20 and its usually 1 or 2. If we're
* not in a test let's log something and round from our best guess.
*/
assert false : String.format(
Locale.ROOT,
"Expected to find the rounding in 100 iterations but didn't for [%d] with [%s]",
utcMillis,
TimeIntervalRounding.this.toString()
);
logger.debug(
"Expected to find the rounding in 100 iterations but didn't for {} using {}",
utcMillis,
TimeIntervalRounding.this.toString()
);
return round(from);
}
}
}

48
server/src/test/java/org/elasticsearch/common/RoundingTests.java
View File

@ -449,13 +449,13 @@ public class RoundingTests extends ESTestCase {
assertThat("Values smaller than rounded value should round further down", prepared.round(roundedDate - 1),
lessThan(roundedDate));
assertThat("Rounding should be >= previous rounding value", roundedDate, greaterThanOrEqualTo(previousRoundedValue));
assertThat("NextRounding value should be greater than date", nextRoundingValue, greaterThan(roundedDate));
assertThat("NextRounding value rounds to itself", nextRoundingValue,
isDate(rounding.round(nextRoundingValue), tz));
if (tz.getRules().isFixedOffset()) {
assertThat("NextRounding value should be greater than date", nextRoundingValue, greaterThan(roundedDate));
assertThat("NextRounding value should be interval from rounded value", nextRoundingValue - roundedDate,
equalTo(interval));
assertThat("NextRounding value should be a rounded date", nextRoundingValue,
equalTo(rounding.round(nextRoundingValue)));
}
previousRoundedValue = roundedDate;
} catch (AssertionError e) {
@ -469,6 +469,48 @@ public class RoundingTests extends ESTestCase {
}
}
/**
* Check a {@link Rounding.Prepared#nextRoundingValue} that was difficult
* to build well with the java.time APIs.
*/
public void testHardNextRoundingValue() {
Rounding rounding = new Rounding.TimeIntervalRounding(960000, ZoneId.of("Europe/Minsk"));
long rounded = rounding.prepareForUnknown().round(877824908400L);
long next = rounding.prepareForUnknown().nextRoundingValue(rounded);
assertThat(next, greaterThan(rounded));
}
/**
* Check a {@link Rounding.Prepared#nextRoundingValue} that was difficult
* to build well with the java.time APIs.
*/
public void testOtherHardNextRoundingValue() {
Rounding rounding = new Rounding.TimeIntervalRounding(480000, ZoneId.of("Portugal"));
long rounded = rounding.prepareJavaTime().round(972780720000L);
long next = rounding.prepareJavaTime().nextRoundingValue(rounded);
assertThat(next, greaterThan(rounded));
}
/**
* Check a {@link Rounding.Prepared#nextRoundingValue} that was difficult
* to build well our janky Newton's Method/binary search hybrid.
*/
public void testHardNewtonsMethod() {
ZoneId tz = ZoneId.of("Asia/Jerusalem");
Rounding rounding = new Rounding.TimeIntervalRounding(19800000, tz);
assertThat(rounding.prepareJavaTime().nextRoundingValue(1824929914182L), isDate(time("2027-10-31T01:30:00", tz), tz));
}
/**
* Check a {@link Rounding.Prepared#nextRoundingValue} that was difficult
* to build well with the java.time APIs.
*/
public void testOtherHardNewtonsMethod() {
ZoneId tz = ZoneId.of("America/Glace_Bay");
Rounding rounding = new Rounding.TimeIntervalRounding(13800000, tz);
assertThat(rounding.prepareJavaTime().nextRoundingValue(1383463147373L), isDate(time("2013-11-03T03:40:00", tz), tz));
}
/**
* Test that rounded values are always greater or equal to last rounded value if date is increasing.
* The example covers an interval around 2011-10-30T02:10:00+01:00, time zone CET, interval: 2700000ms