elasticmachine
commented
2 years ago Pinging @elastic/es-search (Team:Search)
Closed pgomulka closed 12 hours ago
elasticmachine
commented
2 years ago Pinging @elastic/es-search (Team:Search)
pgomulka
commented
2 years ago the tests uses from = {ZonedDateTime@5468} "1970-01-03T03:46:54Z" to = {ZonedDateTime@5469} "1970-01-03T03:46:54.001Z"
it rounds up from -> 1970-01-03T03:46:54.999999999Z - and I think this might be a bug. If we used "1970-01-03T03:46:54.000Z" it would be fine.
and it rounds to -> 1970-01-03T03:46:54.001Z
it converts this to epoch: low = {Long@7027} 186414999 high = {Long@7028} 186414001
and low > high which causes an exception
elasticmachine
commented
2 years ago Pinging @elastic/es-core-infra (Team:Core/Infra)
cbuescher
commented
2 years ago I think #86508 is caused by the same problem as this test failure, although its happening in "testFromLargerToErrors". The failures can be reproduced by picking the "DATE" field test type and provoking a "from" value that is 1ms into the next second, e.g. by modifying the test like so:
--- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java
@@ -54,7 +54,7 @@ public class RangeFieldTypeTests extends FieldTypeTestCase {
@Before
public void setupProperties() {
- type = randomFrom(RangeType.values());
+ type = RangeType.DATE; //randomFrom(RangeType.values());
nowInMillis = randomNonNegativeLong();
}
@@ -155,7 +155,7 @@ public class RangeFieldTypeTests extends FieldTypeTestCase {
break;
}
case DATE: {
- long fromValue = randomInt();
+ long fromValue = (Math.round(randomInt()/1000)*1000) + 1;
from = ZonedDateTime.ofInstant(Instant.ofEpochMilli(fromValue), ZoneOffset.UTC);
to = ZonedDateTime.ofInstant(Instant.ofEpochMilli(fromValue - 1), ZoneOffset.UTC);
break;I think this is a genuine bug that might habe been introduced by some changed in date rounding on the 8.x line. The same setup still passes on 7.17 for me. The bug should show up rarely but probably also goes unnoticed in practice, since it only gets triggered by this relatively rare setup.
pugnascotia
commented
2 years ago Another failure:
ywelsch
commented
2 years ago I'm removing the search label as this looks to be related to JavaDateMathParser's rounding, which is not owned by the search team.
albertzaharovits
commented
1 year ago Another one that reproduces: https://gradle-enterprise.elastic.co/s/xd75tndam7yrc
./gradlew ':server:test' --tests "org.elasticsearch.index.mapper.RangeFieldTypeTests.testFromLargerToErrors" -Dtests.seed=149B9A22D5DE2C10 -Dtests.locale=hi -Dtests.timezone=Europe/Saratov -Druntime.java=17 -Dtests.fips.enabled=trueGoing to mute
benwtrent
commented
1 year ago This issue occurred again.
./gradlew ':server:test' --tests "org.elasticsearch.index.mapper.RangeFieldTypeTests.testRangeQueryIntersectsAdjacentValues" -Dtests.seed=1B8724C3D83D4334 -Dtests.locale=ru -Dtests.timezone=Canada/Central -Druntime.java=20Note, the parameters are:
fromValue: -1018255000The cause is the date field parsing for the instant. It is losing some accuracy even though the instances are correctly 1 second apart (and in the correct order...)
Gonna mute testRangeQueryIntersectsAdjacentValues as it was forgotten for some reason.
lkts
commented
5 months ago I noticed those mutes while doing other work and took a look (specifically at testRangeQueryIntersectsAdjacentValues).
This happens because of the usage of roundupParser which is enabled here https://github.com/elastic/elasticsearch/blob/a7119a38674d32d5acc2e746d72b922b93802f3a/server/src/main/java/org/elasticsearch/index/mapper/RangeType.java#L288
The problem is that roundupParser does not work on millisecond level, it will round up the entire second by setting nanos to 999_999_999. This is different from the logic of parsing/storing bounds where we store epoch milliseconds and therefore adjust bounds by a millisecond in a gt case to produce a gte. Test testRangeQueryIntersectsAdjacentValues seems to be aware of that and rightfully expects bounds that are 1 millisecond apart to work.
The rounding logic was introduced in #50237 which points to documentation saying that gt "Rounds up to the first millisecond not covered by the rounded date.". The last statement describes exactly what we need but it does not seem to be true even though it is documented. Presumably it was broken all the way in #37604?
So to me it does seem like there is something going on with date time parsing here.
elasticsearchmachine
commented
12 hours ago This issue has been closed because it has been open for too long with no activity.
Any muted tests that were associated with this issue have been unmuted.
If the tests begin failing again, a new issue will be opened, and they may be muted again.
Build scan: https://gradle-enterprise.elastic.co/s/2vshbqrk26hss/tests/:server:test/org.elasticsearch.index.mapper.RangeFieldTypeTests/testRangeQueryIntersectsAdjacentValues
Reproduction line:
./gradlew ':server:test' --tests "org.elasticsearch.index.mapper.RangeFieldTypeTests.testRangeQueryIntersectsAdjacentValues" -Dtests.seed=9F5B6EA0226718D0 -Dtests.locale=en-CA -Dtests.timezone=America/Port-au-Prince -Druntime.java=17Applicable branches: 8.2
Reproduces locally?: Yes
Failure history: https://gradle-enterprise.elastic.co/scans/tests?tests.container=org.elasticsearch.index.mapper.RangeFieldTypeTests&tests.test=testRangeQueryIntersectsAdjacentValues
Failure excerpt: