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

Add test for component templates updated after cluster restart #58883

Merged
merged 1 commit into from
Jul 2, 2020

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Jul 1, 2020

This commit adds an integration test that component templates used to form a composite template can
still be updated after a cluster restart.

In #58643 an issue arose where mappings were causing problems because of the way we unwrap _doc in
template mappings. This was also related to the mappings being merged manually rather than using the
MapperService to do the merging. #58643 was fixed in 7.9 and master with the #58521 change, since
mappings now are read and digested by the actual mapper service.

This test passes for 7.x and master, and I intend to open a separate PR including this test for
7.8.1 along with a bug fix for #58643. This test is to ensure we don't have any regression in the
future.

This commit adds an integration test that component templates used to form a composite template can
still be updated after a cluster restart.

In elastic#58643 an issue arose where mappings were causing problems because of the way we unwrap `_doc` in
template mappings. This was also related to the mappings being merged manually rather than using the
`MapperService` to do the merging. elastic#58643 was fixed in 7.9 and master with the elastic#58521 change, since
mappings now are read and digested by the actual mapper service.

This test passes for 7.x and master, and I intend to open a separate PR including this test for
7.8.1 along with a bug fix for elastic#58643. This test is to ensure we don't have any regression in the
future.
@dakrone dakrone added >test Issues or PRs that are addressing/adding tests :Data Management/Indices APIs APIs to create and manage indices and templates v8.0.0 v7.9.0 labels Jul 1, 2020
@dakrone dakrone requested a review from andreidan July 1, 2020 23:11
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Indices APIs)

@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Jul 1, 2020
Copy link
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Lee

@dakrone dakrone merged commit 817dfa6 into elastic:master Jul 2, 2020
@dakrone dakrone deleted the itv2-add-restart-test branch July 2, 2020 13:13
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Jul 2, 2020
…ic#58883)

This commit adds an integration test that component templates used to form a composite template can
still be updated after a cluster restart.

In elastic#58643 an issue arose where mappings were causing problems because of the way we unwrap `_doc` in
template mappings. This was also related to the mappings being merged manually rather than using the
`MapperService` to do the merging. elastic#58643 was fixed in 7.9 and master with the elastic#58521 change, since
mappings now are read and digested by the actual mapper service.

This test passes for 7.x and master, and I intend to open a separate PR including this test for
7.8.1 along with a bug fix for elastic#58643. This test is to ensure we don't have any regression in the
future.
dakrone added a commit that referenced this pull request Jul 2, 2020
…58883) (#58914)

This commit adds an integration test that component templates used to form a composite template can
still be updated after a cluster restart.

In #58643 an issue arose where mappings were causing problems because of the way we unwrap `_doc` in
template mappings. This was also related to the mappings being merged manually rather than using the
`MapperService` to do the merging. #58643 was fixed in 7.9 and master with the #58521 change, since
mappings now are read and digested by the actual mapper service.

This test passes for 7.x and master, and I intend to open a separate PR including this test for
7.8.1 along with a bug fix for #58643. This test is to ensure we don't have any regression in the
future.
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Jul 2, 2020
Due to the way that our `toXContent` of mappings works, we reduce the type to remove `_doc` from
template mappings when serializing them to disk as part of cluster state. This causes problems
however, as we always expected there to be a `_doc` field in the mappings. This was resolved in
later versions of ES (7.9+) by elastic#58521 which uses a real mapper to handle the mappings. For 7.8.1,
however, we need to ensure that we can still read the state after it has been persisted to disk.

Resolves elastic#58643
Relates to elastic#58883
Relates to elastic#58521
dakrone added a commit that referenced this pull request Jul 6, 2020
Due to the way that our `toXContent` of mappings works, we reduce the type to remove `_doc` from
template mappings when serializing them to disk as part of cluster state. This causes problems
however, as we always expected there to be a `_doc` field in the mappings. This was resolved in
later versions of ES (7.9+) by #58521 which uses a real mapper to handle the mappings. For 7.8.1,
however, we need to ensure that we can still read the state after it has been persisted to disk.

Resolves #58643
Relates to #58883
Relates to #58521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v7.9.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants