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

Date histogram bwc deprecation failure #42258

Closed
rjernst opened this issue May 21, 2019 · 12 comments · Fixed by #42290
Closed

Date histogram bwc deprecation failure #42258

rjernst opened this issue May 21, 2019 · 12 comments · Fixed by #42290
Assignees
Labels
:Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test-failure Triaged test failures from CI

Comments

@rjernst
Copy link
Member

rjernst commented May 21, 2019

Intake got a bwc test failure, which reproduces:

./gradlew ":x-pack:qa:rolling-upgrade:v7.2.0#oldClusterTestRunner" --tests "org.elasticsearch.upgrades.UpgradeClusterClientYamlTestSuiteIT.test {p0=old_cluster/40_ml_datafeed_crud/Put job and datafeed with aggs in old cluster - pre-deprecated interval}" -Dtests.seed=BF350E5616C9D3D5 -Dtests.security.manager=true -Dtests.locale=en-SC -Dtests.timezone=Asia/Nicosia -Dcompiler.java=12 -Druntime.java=11 -Dtests.rest.suite=old_cluster

The underlying failure is:

 2> java.lang.AssertionError: Failure at [old_cluster/40_ml_datafeed_crud:75]: got unexpected warning header [
        299 Elasticsearch-7.2.0-SNAPSHOT-b0a25c3 "[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."
    ]

@rjernst rjernst added :Analytics/Aggregations Aggregations >test-failure Triaged test failures from CI labels May 21, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@rjernst
Copy link
Member Author

rjernst commented May 21, 2019

@polyfractal this looks related to #41906, can you take a look?

@polyfractal polyfractal self-assigned this May 21, 2019
polyfractal added a commit that referenced this issue May 21, 2019
@polyfractal
Copy link
Contributor

Muted in 570e8ed

I still need to update the versions after backport, and most of them are ok because they are skip: version: - 8.0.0, but this test is the other way around with skip: version: 8.0.0 - . So as soon as the 7.x backport landed it started failing.

I'd prefer to fix all those versions in one commit, so just muting this for now.

polyfractal added a commit that referenced this issue May 21, 2019
polyfractal added a commit that referenced this issue May 22, 2019
AwaitsFix #42258

Thought this was fixed, but throwing deprecation warnings at
an unexpected time so putting this back on mute until we
figure it out.
@polyfractal
Copy link
Contributor

Reopening. The version issue is fixed, but the test itself is throwing an unexpected deprecation warning somewhere (either uncaught, or multiples when only one was expected). Investigating

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+intake/3658/testReport/junit/org.elasticsearch.upgrades/UpgradeClusterClientYamlTestSuiteIT/test__p0_mixed_cluster_40_ml_datafeed_crud_Put_job_and_datafeed_with_aggs_in_mixed_cluster_/

./gradlew :x-pack:qa:rolling-upgrade:v7.2.0#twoThirdsUpgradedTestRunner --tests "org.elasticsearch.upgrades.UpgradeClusterClientYamlTestSuiteIT.test {p0=mixed_cluster/40_ml_datafeed_crud/Put job and datafeed with aggs in mixed cluster}" -Dtests.seed=E889580741B6A09C -Dtests.security.manager=true -Dtests.locale=ar-001 -Dtests.timezone=Indian/Cocos -Dcompiler.java=12 -Druntime.java=11 -Dtests.rest.suite=mixed_cluster

@polyfractal polyfractal reopened this May 22, 2019
@tvernum
Copy link
Contributor

tvernum commented May 23, 2019

Also failing in 7.x

org.elasticsearch.upgrades.UpgradeClusterClientYamlTestSuiteIT test {p0=old_cluster/40_ml_datafeed_crud/Put job and datafeed with aggs in old cluster - pre-deprecated interval}

REPRODUCE WITH: ./gradlew :x-pack:qa:rolling-upgrade:v7.2.0#oldClusterTestRunner --tests "org.elasticsearch.upgrades.UpgradeClusterClientYamlTestSuiteIT.test {p0=old_cluster/40_ml_datafeed_crud/Put job and datafeed with aggs in old cluster - pre-deprecated interval}" \
  -Dtests.seed=E41C850A2FBBA00C \
  -Dtests.security.manager=true \
  -Dtests.locale=en-ZA \
  -Dtests.timezone=Asia/Kathmandu \
  -Dcompiler.java=12 \
  -Druntime.java=8 \
  -Dtests.rest.suite=old_cluster

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+7.x+periodic/578/console

polyfractal added a commit that referenced this issue May 23, 2019
AwaitsFix #42258

Thought this was fixed, but throwing deprecation warnings at
an unexpected time so putting this back on mute until we
figure it out.
@polyfractal
Copy link
Contributor

Pushed a mute to 7.x as well.

Will try to figure out what's going on with this test today so we can get it fixed/unmuted

gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
AwaitsFix elastic#42258

Thought this was fixed, but throwing deprecation warnings at
an unexpected time so putting this back on mute until we
figure it out.
@polyfractal
Copy link
Contributor

Hmm, tried to get this fixed and think I'm going to need some help. @dimitris-athanasiou, it seems that the mixed cluster 40_ml_datafeed_crud#Test Old Cluster with Aggs test throws either one or two deprecation warnings, which means we can't stably test it in the yaml tests since it appears random.

Do you know why it might change from seed to seed? Any ideas how to get these tests fixed and unmuted?

@davidkyle
Copy link
Member

This issue should be closed by #42290 but there has been 2 recent failures on the 7.2 branch

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+7.2+bwc-tests/86/console
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+7.2+bwc-tests/87/console

Failure at [old_cluster/40_ml_datafeed_crud:75]: got unexpected warning header [
	299 Elasticsearch-7.2.0-508c38a "[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future."
]

The skip needs to be updated for version 7.2.0 as in a rolling upgrade to 7.2.1 the old cluster can be 7.2.0 which will return the deprecation warning. @polyfractal you fixed this on the other branches can you take a look please.

@polyfractal
Copy link
Contributor

Will mute on 7.2 in a minute.

#42290 doesn't fix the failure though, because the root issue is that the test throws one or two deprecation warnings and I'm not sure why yet. You can see that the 7.2 test is already expecting a deprecation warning, the problem is that a second unexpected (but identical) deprecation warning is thrown. Dimitris noticed that when two deprecations are thrown they are coming from different nodes which is likely part of it.

I think the bigger issue is that the yaml rest rolling restart tests don't have a good way to know which node we're talking to, so when we're in a mixed cluster situation it's not clear if we are talking to someone pre- or post-upgrade... so it's impossible to know which deprecations will or won't be thrown. And I think the version skip is based on checkout branch not the bwc-version branch.

Finally, if we mute just one test it fails the entire suite because they rely on each other (570e8ed attempted to mute just the offending test, but that breaks the entire rolling upgrade suite)

It's also possible I'm missing something, but haven't found a workaround yet.

polyfractal added a commit that referenced this issue Jun 27, 2019
AwaitsFix #42258

Thought this was fixed, but throwing deprecation warnings at
an unexpected time so putting this back on mute until we
figure it out.
@hendrikmuhs
Copy link

Some note as I am working on a similar problem (upgrade tests for deprecated API's):

Type removal solved this problem by ignoring warnings:

Not the most elegant way but pragmatic and quickly implemented.

@rjernst rjernst added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 4, 2020
@talevy
Copy link
Contributor

talevy commented Jul 14, 2020

it seems like the note Hendrik left should solve this issue moving forward. Think we can close this one @polyfractal?

@polyfractal
Copy link
Contributor

This appears to have been fixed in #52538

Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test-failure Triaged test failures from CI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants