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] Occasional failures WatchStatusIntegrationTests#testThatStatusGetsUpdated #38619

Closed
cbuescher opened this issue Feb 8, 2019 · 4 comments · Fixed by #38626
Closed

[CI] Occasional failures WatchStatusIntegrationTests#testThatStatusGetsUpdated #38619

cbuescher opened this issue Feb 8, 2019 · 4 comments · Fixed by #38626

Comments

@cbuescher
Copy link
Member

I got this failure locally on master, it reproduces after a few iterations with:

./gradlew :x-pack:plugin:watcher:unitTest -Dtests.seed=A92DF59B55106F76 -Dtests.class=org.elasticsearch.xpack.watcher.watch.WatchStatusIntegrationTests -Dtests.method="testThatStatusGetsUpdated" -Dtests.security.manager=true -Dtests.locale=en-BW -Dtests.timezone=SystemV/YST9 -Dcompiler.java=11 -Druntime.java=11

The error is:

FAILURE 4.67s | WatchStatusIntegrationTests.testThatStatusGetsUpdated <<< FAILURES!
   > Throwable #1: java.lang.AssertionError:
   > Expected: is "2019-02-08T12:25:25.22141Z"
   >      but: was "2019-02-08T12:25:25.221410Z"
   >    at __randomizedtesting.SeedInfo.seed([A92DF59B55106F76:E51670CD660E2260]:0)
   >    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
   >    at org.elasticsearch.xpack.watcher.watch.WatchStatusIntegrationTests.testThatStatusGetsUpdated(WatchStatusIntegrationTests.java:49)
   >    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   >    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   >    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   >    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   >    at java.base/java.lang.Thread.run(Thread.java:834)

This looks like another of the cases where the new java time formatting swallows trailing zeros. @pgomulka could you take a look, I remember reviewing #38505 which looked similar.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Feb 8, 2019
cbuescher pushed a commit that referenced this issue Feb 8, 2019
cbuescher pushed a commit that referenced this issue Feb 8, 2019
cbuescher pushed a commit that referenced this issue Feb 8, 2019
@cbuescher
Copy link
Member Author

Muted on master, 7.x (779673c) and 7.0 (cfde3de)

@pgomulka
Copy link
Contributor

pgomulka commented Feb 8, 2019

interestingly WatchStatus.lastChecked or WatchStatus.lastMetCondition are using a different formatting then WatchStatus.state.timestamp when converting toXcontent

The lastChecked and lastMetCondition are formatted using XContentElasticsearchExtensions

        transformers.put(ZonedDateTime.class, d -> DEFAULT_FORMATTER.format((ZonedDateTime) d));

where DEFAULT_FORMATTER is strict_date_optional_time_nanos

Whereas WatchStatus.state.timestamp is using:

WatcherDateTimeUtils.writeDate(Field.TIMESTAMP.getPreferredName(), builder, timestamp)

with "strict_date_optional_time||epoch_millis"

These are different for instance when formatting 2019-02-08T13:37:06.**687670Z**
WatcherDateTimeUtils.dateTimeFormatter -> 2019-02-08T13:37:06.687Z
XContentElasticsearchExtension.DEFAULT_FORMATTER -> 2019-02-08T13:37:06.68767Z

@cbuescher @spinscale Do you think this is intentional?

@spinscale
Copy link
Contributor

on top of my head I'd say we keep all on milliseconds, there is no need for any nanosecond granularity in watcher, as the date field is only in millisecond resolution anyway.

pgomulka added a commit that referenced this issue Feb 12, 2019
Change the formatting for Watcher.status.lastCheck and lastMetCondition
to be the same as Watcher.status.state.timestamp. These should all have
only millisecond precision

closes #38619
pgomulka added a commit to pgomulka/elasticsearch that referenced this issue Feb 12, 2019
Change the formatting for Watcher.status.lastCheck and lastMetCondition
to be the same as Watcher.status.state.timestamp. These should all have
only millisecond precision

closes elastic#38619
pgomulka added a commit to pgomulka/elasticsearch that referenced this issue Feb 12, 2019
Change the formatting for Watcher.status.lastCheck and lastMetCondition
to be the same as Watcher.status.state.timestamp. These should all have
only millisecond precision

closes elastic#38619
pgomulka added a commit that referenced this issue Feb 13, 2019
…ort#38626

Change the formatting for Watcher.status.lastCheck and lastMetCondition
to be the same as Watcher.status.state.timestamp. These should all have
only millisecond precision
closes #38619
backport #38626
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants