Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI] TimeZoneRoundingTests#testRoundingRandom fails #27966

Closed
davidkyle opened this issue Dec 22, 2017 · 5 comments
Closed

[CI] TimeZoneRoundingTests#testRoundingRandom fails #27966

davidkyle opened this issue Dec 22, 2017 · 5 comments
Assignees
Labels
:Analytics/Aggregations Aggregations >bug >test-failure Triaged test failures from CI v7.0.0-beta1

Comments

@davidkyle
Copy link
Member

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+5.6+periodic/1149/console

   > Throwable #1: java.lang.AssertionError
   >    at __randomizedtesting.SeedInfo.seed([1E275DAB660CE5C0:B169E5B952C9109D]:0)
   >    at org.elasticsearch.common.rounding.TimeZoneRoundingTests.assertInterval(TimeZoneRoundingTests.java:589)
   >    at org.elasticsearch.common.rounding.TimeZoneRoundingTests.testRoundingRandom(TimeZoneRoundingTests.java:196)

This is a reproducible failure:

gradle :core:test \
  -Dtests.seed=1E275DAB660CE5C0 \
  -Dtests.class=org.elasticsearch.common.rounding.TimeZoneRoundingTests \
  -Dtests.method="testRoundingRandom" \
  -Dtests.security.manager=true \
  -Dtests.locale=et-EE \
  -Dtests.timezone=America/Grand_Turk
@davidkyle davidkyle added the >test-failure Triaged test failures from CI label Dec 22, 2017
@DaveCTurner
Copy link
Contributor

DaveCTurner commented Dec 22, 2017

This reproduces in master (7b7c8c76a76ecb12e60f239ad7a50fe71ea7c149) too.

The offending time zone is America/St_Johns and the failing test is testing rounding in days - specifically rounding the chosen time point 1162091532554ms = 2006-10-29T03:12:12.554Z up and down to the nearest day and checking that the chosen time is between the rounded times.

It isn't, however. It rounds down to 1162002600000ms = 2006-10-28T02:30:00Z and rounds up to 1162089000000ms = 2006-10-29T02:30:00Z, both of which are earlier than the chosen time point.

Of importance, surely, is that America/St_Johns changed their clocks at 2006-10-29T00:01 (local time) from UTC+02:30 to UTC+03:30. The rounded time points are midnight local time as expected.

@DaveCTurner
Copy link
Contributor

Note to future self: because this time zone sets the clocks back at 00:01, it has the first minute of Sunday 2006-10-29 followed by 59 bonus minutes of Saturday 2006-10-28, and the chosen time is within that bonus period. The bug seems to be that we expect every time on a day to precede the following day's midnight within a given timezone, which is one of many reasonable-yet-false assumptions you can make about how time works.

@DaveCTurner DaveCTurner added :Analytics/Aggregations Aggregations and removed >test-failure Triaged test failures from CI labels Jan 3, 2018
@DaveCTurner
Copy link
Contributor

DaveCTurner commented Jan 8, 2018

We concluded in today's Search & Aggs meeting that the current behaviour is desirable and the assertion in the test is at fault. The fix is therefore to change the assertion.

As of TZDB 2017c, the only regular occurrences of these overlapping days since 1950 are the clocks going back in America/St_Johns (Newfoundland, 1987-2010), America/Goose_Bay (Labrador, 1987-2010) and America/Moncton (New Brunswick, 1993-2006) as described above. As a further bonus, in America/St_Johns in 1988 the clocks actually went back 2 hours at 00:01. Eastern Canada is a special place indeed.

There was also a one-off shift in Antarctica/Casey by three hours from UTC+11 to UTC+8 at 2010-03-05 02:00 local time which took it back to 2010-03-04 for an extra hour.

@DaveCTurner
Copy link
Contributor

(also note that Canada/Newfoundland is an alias to America/St_Johns so suppress that too)

@clintongormley clintongormley added the >test-failure Triaged test failures from CI label Jan 11, 2018
@DaveCTurner
Copy link
Contributor

This will be fixed by #28151.

DaveCTurner added a commit that referenced this issue Jan 31, 2018
Sometimes, in some places, the clocks are set back across midnight, leading to
overlapping days. This was not handled as expected, and this change fixes this.

Additionally, in this situation it is not true that rounding a time down to the
nearest day is a monotonic operation, as asserted in these tests. This change 
suppresses those assertions in those rare cases.

Fixes #27966.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >bug >test-failure Triaged test failures from CI v7.0.0-beta1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants