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

Fixes to task result index mapping #50359

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

romseygeek
Copy link
Contributor

@romseygeek romseygeek commented Dec 19, 2019

The build-in mapping for the tasks result index still has a mapping type defined; while
this does not matter for index creation, as we still have a create method that takes a
top-level type, it does matter for updates. In combination with a separate bug, that the
built-in mapping has not incremented its meta version, this meant that tasks submitted
to a cluster with an already existing task index would attempt to update the mappings
on that index, and fail due to the top-level type.

This commit fixes the mapping to have a top-level mapping of _doc, and also updates
the meta version so that we do not update mappings on every new task. It also adds a
test that explicitly runs two asynchronous tasks to ensure that the mappings do not
cause a failure.

Fixes #50248

@romseygeek romseygeek added >bug :Search Foundations/Mapping Index mappings, including merging and defining field types :Distributed Coordination/Task Management Issues for anything around the Tasks API - both persistent and node level. v8.0.0 labels Dec 19, 2019
@romseygeek romseygeek requested a review from jimczi December 19, 2019 10:35
@romseygeek romseygeek self-assigned this Dec 19, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Mapping)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Task Management)

@romseygeek
Copy link
Contributor Author

Note that while the bug itself is not released, 7.5 contains the error in the mapping version which means that it will be applying a mapping update on every new task. I'll open a separate bugfix PR to correct this.

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM

@romseygeek romseygeek merged commit 2ee4f1b into elastic:master Dec 19, 2019
romseygeek added a commit that referenced this pull request Jan 2, 2020
)

We still have a number of places, mainly in test code but some in production, that
are building mappings with a named type as the root of a map. CreateIndexRequest
handles this automatically, but PutMappingRequest does not, which is a bit trappy -
we can get situations like #50359 where the same mapping will work when an
index is created but fail on an update.

This commit is a first step to removing the leniency in CreateIndexRequest so that
we can catch mappings with a named type root earlier.

Relates to #41059
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
The built-in mapping for the tasks result index still has a mapping type defined; while
this does not matter for index creation, as we still have a create method that takes a
top-level type, it does matter for updates. In combination with a separate bug, that the
built-in mapping has not incremented its meta version, this meant that tasks submitted
to a cluster with an already existing task index would attempt to update the mappings
on that index, and fail due to the top-level type.

This commit fixes the mapping to have a top-level mapping of _doc, and also updates
the meta version so that we do not update mappings on every new task. It also adds a
test that explicitly runs two asynchronous tasks to ensure that the mappings do not
cause a failure.

Fixes elastic#50248
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
…stic#50419)

We still have a number of places, mainly in test code but some in production, that
are building mappings with a named type as the root of a map. CreateIndexRequest
handles this automatically, but PutMappingRequest does not, which is a bit trappy -
we can get situations like elastic#50359 where the same mapping will work when an
index is created but fail on an update.

This commit is a first step to removing the leniency in CreateIndexRequest so that
we can catch mappings with a named type root earlier.

Relates to elastic#41059
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed Coordination/Task Management Issues for anything around the Tasks API - both persistent and node level. :Search Foundations/Mapping Index mappings, including merging and defining field types v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reindex wait_for_completion=false task failure
4 participants