TSDB: fix reindex failed tests without feature flag by weizijun · Pull Request #81967 · elastic/elasticsearch

weizijun · 2021-12-21T02:37:35Z

fix as the #80945 do.

register a settings update consumer for the end_time for
the tsdb index even when the end_time setting wasn't registered.
Pass the feature flag to reindex yaml tests.

weizijun · 2021-12-21T02:40:05Z

hi, @imotov , I fix the tests in #81916

elasticmachine · 2021-12-21T04:51:57Z

Pinging @elastic/es-analytics-geo (Team:Analytics)

imotov · 2021-12-21T04:52:26Z

@elasticmachine test this please

nik9000 · 2021-12-21T05:01:12Z

Lgtm

…

On Mon, Dec 20, 2021, 11:52 PM Igor Motov ***@***.***> wrote: @elasticmachine <https://github.com/elasticmachine> test this please — Reply to this email directly, view it on GitHub <#81967 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABUXISGIZBP5IFHHBKMKSTUSABZLANCNFSM5KPC4QCQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are on a team that was mentioned.Message ID: ***@***.***>

imotov

Changes in gradle fix the issue I don't think change in TimestampBounds are necessary.

imotov · 2021-12-21T17:01:46Z

server/src/main/java/org/elasticsearch/index/TimestampBounds.java

-        scopedSettings.addSettingsUpdateConsumer(IndexSettings.TIME_SERIES_END_TIME, this::updateEndTime);
+        if (IndexSettings.isTimeSeriesModeEnabled()) {
+            scopedSettings.addSettingsUpdateConsumer(IndexSettings.TIME_SERIES_END_TIME, this::updateEndTime);
+        }


Not sure why this is necessary. It seems like TimestampBounds instantiation is already guarded by check for the TIME_SERIES_START_TIME in

elasticsearch/server/src/main/java/org/elasticsearch/index/IndexSettings.java

Line 710 in 3e664d3

this.timestampBounds = TIME_SERIES_START_TIME.exists(settings) ? new TimestampBounds(scopedSettings) : null;

I think @nik9000 had to add this check in #80945 because there was no protection there.
So

@weizijun could you revert this bit? Or explain why it's needed. Like @imotov says I think I think we won't make a TimestampBounds at all without the feature flag.

But I've been known to be wrong about things.

@imotov @nik9000 sorry, I see it just now, I see igor has removed the code

nik9000

Left a note about the change in the java file.

mark-vieira · 2022-01-05T21:05:10Z

@elasticmachine update branch

mark-vieira · 2022-01-05T21:05:30Z

@elasticmachine test this please

mark-vieira · 2022-01-05T21:06:06Z

@imotov @nik9000 can we maybe just make the necessary changes here to get this merged. As it is our release test builds have been busted for some time.

imotov · 2022-01-05T21:15:09Z

@mark-vieira my apologizes, will not happen again. I am on it.

mark-vieira · 2022-01-05T21:16:37Z

No worries. Stuff gets lost in the shuffle over the holidays. Thanks, Igor!

imotov · 2022-01-05T21:26:17Z

@elasticmachine test this please

imotov · 2022-01-05T22:12:53Z

@elasticmachine test this please

imotov · 2022-01-05T22:59:42Z

@elasticmachine run elasticsearch-ci/release-tests

imotov · 2022-01-06T01:07:35Z

@elasticmachine run elasticsearch-ci/release-tests

imotov · 2022-01-06T01:09:03Z

@elasticmachine run elasticsearch-ci/release-tests

imotov · 2022-01-06T02:26:31Z

@elasticmachine run elasticsearch-ci/release-tests

imotov · 2022-01-06T04:40:38Z

@elasticmachine test this please

imotov · 2022-01-06T06:09:23Z

@elasticmachine run elasticsearch-ci/release-tests

weizijun · 2022-01-06T11:11:03Z

@elasticmachine update branch

weizijun · 2022-01-06T11:16:14Z

there is another failed test, cause the release check failed:

 ./gradlew ':x-pack:plugin:ml:qa:native-multi-node-tests:javaRestTest' --tests "org.elasticsearch.xpack.ml.integration.PyTorchModelIT.testGetDeploymentStats_WithStartedStoppedDeployments" -Dtests.seed=5E2AFE7B6589657 -Dbuild.snapshot=false -Dtests.jvm.argline="-Dbuild.snapshot=false" -Dtests.locale=ar-TN -Dtests.timezone=America/Campo_Grande -Druntime.java=17

nik9000 · 2022-01-06T14:47:06Z

@elasticmachine, test this please.

nik9000 · 2022-01-06T14:49:44Z

I've removed the test-release flag from this PR. I'll look into that failure and open a follow up PR to fix the release tests. It's nice to have the bot's full trust for fixing that because the build will fail over and over and over and over again....

nik9000 · 2022-01-06T17:28:13Z

I've removed the test-release flag from this PR. I'll look into that failure and open a follow up PR to fix the release tests. It's nice to have the bot's full trust for fixing that because the build will fail over and over and over and over ag

Removing it doesn't stop the tests! Nice. Oh well. I have to do a thing for a bit. I'll look again soon....

nik9000 · 2022-01-06T19:44:22Z

This error isn't ours!

» [2022-01-06T14:48:38,148][ERROR][o.e.x.m.i.p.p.PyTorchResultProcessor] [javaRestTest-2] [foo] Error processing results
»  org.elasticsearch.xcontent.XContentParseException: [2:3] [pytorch_result] unknown field [request_id]
»       at org.elasticsearch.xcontent.ObjectParser.lambda$errorOnUnknown$2(ObjectParser.java:104) ~[elasticsearch-x-content-8.1.0.jar:8.1.0]
»       at org.elasticsearch.xcontent.ObjectParser.parse(ObjectParser.java:316) ~[elasticsearch-x-content-8.1.0.jar:8.1.0]
»       at org.elasticsearch.xcontent.ConstructingObjectParser.parse(ConstructingObjectParser.java:166) ~[elasticsearch-x-content-8.1.0.jar:8.1.0]
»       at org.elasticsearch.xcontent.ConstructingObjectParser.apply(ConstructingObjectParser.java:158) ~[elasticsearch-x-content-8.1.0.jar:8.1.0]
»       at org.elasticsearch.xpack.ml.process.ProcessResultsParser$ResultIterator.next(ProcessResultsParser.java:86) ~[x-pack-ml-8.1.0.jar:8.1.0]
»       at org.elasticsearch.xpack.ml.inference.pytorch.process.PyTorchResultProcessor.process(PyTorchResultProcessor.java:62) [x-pack-ml-8.1.0.jar:8.1.0]
»       at org.elasticsearch.xpack.ml.inference.deployment.DeploymentManager.lambda$doStartDeployment$2(DeploymentManager.java:135) [x-pack-ml-8.1.0.jar:8.1.0]
»       at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:716) [elasticsearch-8.1.0.jar:8.1.0]
»       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
»       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
»       at java.lang.Thread.run(Thread.java:833) [?:?]

I'm going to merge this and ping the ml folks with this error.

nik9000 · 2022-01-06T19:44:41Z

I reproduced with:

./gradlew ':x-pack:plugin:ml:qa:native-multi-node-tests:javaRestTest' --tests "org.elasticsearch.xpack.ml.integration.PyTorchModelIT.testGetDeploymentStats_WithStartedStoppedDeployments" -Dtests.seed=5E2AFE7B6589657 -Dbuild.snapshot=false -Dtests.jvm.argline="-Dbuild.snapshot=false" -Dtests.locale=ar-TN -Dtests.timezone=America/Campo_Grande -Druntime.java=17 -Dlicense.key="x-pack/license-tools/src/test/resources/public.key"

weizijun · 2022-01-07T03:33:11Z

thanks @imotov @nik9000

fix as the elastic#80945 do. register a settings update consumer for the end_time for the tsdb index even when the end_time setting wasn't registered. Pass the feature flag to reindex yaml tests. Co-authored-by: Igor Motov <igor@motovs.org>

fixup

f2ae031

elasticsearchmachine added v8.1.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Dec 21, 2021

fixup

02872df

imotov added :StorageEngine/TSDB You know, for Metrics >bug labels Dec 21, 2021

elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Dec 21, 2021

imotov self-requested a review December 21, 2021 05:00

imotov reviewed Dec 21, 2021

View reviewed changes

nik9000 requested changes Jan 5, 2022

View reviewed changes

mark-vieira added the test-release Trigger CI checks against release build label Jan 5, 2022

Merge branch 'master' into fix-tsdb-flag-error

91b2f17

Remove unnecessary isTimeSeriesModeEnabled check

0887ffb

Fix analysis-icu:test

d8e7aa6

Fix mapper-murmur3:test

c325517

Fix mapper-annotated-text:internalClusterTest

f4c69e7

Fix x-pack:plugin:analytics:internalClusterTest

f8baa92

Fix tests in other mapper's plugins

5fe6e43

Fix rest tests in mapper-unsigned-long

d25f130

Merge branch 'master' into fix-tsdb-flag-error

9deb8f3

nik9000 removed the test-release Trigger CI checks against release build label Jan 6, 2022

nik9000 approved these changes Jan 6, 2022

View reviewed changes

nik9000 added auto-backport-and-merge auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Jan 6, 2022

nik9000 merged commit b6e8b59 into elastic:master Jan 6, 2022

nik9000 mentioned this pull request Jan 6, 2022

[CI] ReindexClientYamlTestSuiteIT test {yaml=delete_by_query/10_basic/tsdb} failing #81916

Closed

pugnascotia added the >test Issues or PRs that are addressing/adding tests label Jan 31, 2022

Conversation

weizijun commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weizijun commented Dec 21, 2021

Uh oh!

elasticmachine commented Dec 21, 2021

Uh oh!

imotov commented Dec 21, 2021

Uh oh!

nik9000 commented Dec 21, 2021 via email

Uh oh!

imotov left a comment

Choose a reason for hiding this comment

Uh oh!

imotov Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

nik9000 Jan 5, 2022

Choose a reason for hiding this comment

Uh oh!

nik9000 Jan 5, 2022

Choose a reason for hiding this comment

Uh oh!

weizijun Jan 6, 2022

Choose a reason for hiding this comment

Uh oh!

nik9000 left a comment

Choose a reason for hiding this comment

Uh oh!

mark-vieira commented Jan 5, 2022

Uh oh!

mark-vieira commented Jan 5, 2022

Uh oh!

mark-vieira commented Jan 5, 2022

Uh oh!

imotov commented Jan 5, 2022

Uh oh!

mark-vieira commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imotov commented Jan 5, 2022

Uh oh!

imotov commented Jan 5, 2022

Uh oh!

imotov commented Jan 5, 2022

Uh oh!

imotov commented Jan 6, 2022

Uh oh!

imotov commented Jan 6, 2022

Uh oh!

imotov commented Jan 6, 2022

Uh oh!

imotov commented Jan 6, 2022

Uh oh!

imotov commented Jan 6, 2022

Uh oh!

weizijun commented Jan 6, 2022

Uh oh!

weizijun commented Jan 6, 2022

Uh oh!

nik9000 commented Jan 6, 2022

Uh oh!

nik9000 commented Jan 6, 2022

Uh oh!

nik9000 commented Jan 6, 2022

Uh oh!

nik9000 commented Jan 6, 2022

Uh oh!

nik9000 commented Jan 6, 2022

Uh oh!

weizijun commented Jan 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

weizijun commented Dec 21, 2021 •

edited

Loading

mark-vieira commented Jan 5, 2022 •

edited

Loading