Skip to content

Runtime fields core-with-mapped tests support tsdb #83577

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

Conversation

weizijun
Copy link
Contributor

@weizijun weizijun commented Feb 7, 2022

As runtime fields not support time_series_dimension and time_series_metric, it will lead to the failure of tsdb test case. And tsdb indices require the @timestamp field.

So I improve the runtimeifyMappingProperties method logic, add some skip rule.

  • skip time_series_dimension field.
  • skip time_series_metric field.
  • skip @timestamp field.

And the PR fixed the failed test in #83431

@elasticsearchmachine elasticsearchmachine added external-contributor Pull request authored by a developer outside the Elasticsearch team v8.2.0 labels Feb 7, 2022
@weizijun weizijun changed the title runtime fields core-with-mapped tests support tsdb Runtime fields core-with-mapped tests support tsdb Feb 7, 2022
@weizijun
Copy link
Contributor Author

weizijun commented Feb 7, 2022

@nik9000 can you help to review this PR?

@nik9000 nik9000 self-requested a review February 7, 2022 16:28
if (name.equals(DataStreamTimestampFieldMapper.DEFAULT_PATH)) {
// time_series and data stream indices need timestamp field
continue;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -101,7 +99,6 @@ subprojects {
'search/330_fetch_fields/error includes glob pattern',
// we need a @timestamp field to be defined in index mapping
'search/380_sort_segments_on_timestamp/*',
'field_caps/40_time_series/*',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is a bit broad. How many are failing without this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The yaml failed because of the setup create index, index mapping has time_series_dimension and time_series_metric.
I remove this line, and run the test, it is ok.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the line from the PR? I think you've covered this in the translater.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, done

@nik9000
Copy link
Member

nik9000 commented Feb 7, 2022

@nik9000 can you help to review this PR?

Yeah! Thanks for the ping. I've replied inline

@nik9000 nik9000 self-assigned this Feb 7, 2022
@nik9000 nik9000 added :StorageEngine/TSDB You know, for Metrics >bug labels Feb 7, 2022
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 7, 2022
@elasticmachine
Copy link
Collaborator

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

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elasticmachine, test this please

@nik9000 nik9000 added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged and removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged labels Feb 8, 2022
@nik9000
Copy link
Member

nik9000 commented Feb 8, 2022

@elasticmachine test this please

@elasticsearchmachine elasticsearchmachine merged commit 9503e9f into elastic:master Feb 8, 2022
@nik9000
Copy link
Member

nik9000 commented Feb 8, 2022

All merged now. Thanks @weizijun !

@weizijun weizijun deleted the runtime-fields-tests-support-tsdb branch February 9, 2022 00:38
@weizijun
Copy link
Contributor Author

weizijun commented Feb 9, 2022

All merged now. Thanks @weizijun !

Thanks @nik9000 !

weizijun added a commit to weizijun/elasticsearch that referenced this pull request Feb 9, 2022
* upstream/master: (166 commits)
  Bind host all instead of just _site_ when needed (elastic#83145)
  [DOCS] Fix min/max agg snippets for histograms (elastic#83695)
  [DOCS] Add deprecation notice for system indices (elastic#83688)
  Cache ILM policy name on IndexMetadata (elastic#83603)
  [DOCS] Fix 8.0 breaking changes sort order (elastic#83685)
  [ML] fix random sampling background query consistency (elastic#83676)
  Move internal APIs into their own namespace '_internal'
  Runtime fields core-with-mapped tests support tsdb (elastic#83577)
  Optimize calculating the presence of a quorum (elastic#83638)
  Use switch expressions in EnableAllocationDecider and NodeShutdownAllocationDecider (elastic#83641)
  Note libffi error message in tmpdir docs (elastic#83662)
  Fix TransportDesiredNodesActionsIT batch tests (elastic#83659)
  [DOCS] Remove unused upgrade doc files (elastic#83617)
  [ML] Wait for model process to stop in stop deployment (elastic#83644)
  [ML] Fix submit after shutdown in process worker service (elastic#83645)
  Remove req/resp classes associated with HLRC (elastic#83599)
  Introduce index.version.compatibility setting (elastic#83264)
  Rename InternalTestCluster#getMasterNodeInstance (elastic#83407)
  Mute TimeSeriesIndexSearcherTests testCollectInOrderAcrossSegments (elastic#83648)
  Add rollover add max_primary_shard_docs condition (elastic#80981)
  ...

# Conflicts:
#	x-pack/plugin/rollup/build.gradle
#	x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug external-contributor Pull request authored by a developer outside the Elasticsearch team :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants