Skip to content

Remove immediate operation retry after mapping update#38873

Merged
dakrone merged 1 commit intoelastic:masterfrom
dakrone:remove-immediate-index-retry
Feb 14, 2019
Merged

Remove immediate operation retry after mapping update#38873
dakrone merged 1 commit intoelastic:masterfrom
dakrone:remove-immediate-index-retry

Conversation

@dakrone
Copy link
Copy Markdown
Member

@dakrone dakrone commented Feb 13, 2019

Prior to this commit, when an indexing operation resulted in an
Engine.Result.Type.MAPPING_UPDATE_REQUIRED, TransportShardBulkAction
immediately retries the indexing operation to see if it succeeds. In the event
that it succeeds the context does not wait until the mapping update has
propagated through the cluster state before finishing the indexing.

In some of our tests we rely on mappings being available as soon as they've been
introduced in a document that indexed correctly. By removing the immediate retry
we always wait for this to be the case.

Resolves #38428
Supercedes #38579
Relates to #38711

@dakrone
Remove immediate operation retry after mapping update
2331751 
Prior to this commit, when an indexing operation resulted in an
`Engine.Result.Type.MAPPING_UPDATE_REQUIRED`, TransportShardBulkAction
immediately retries the indexing operation to see if it succeeds. In the event
that it succeeds the context does not wait until the mapping update has
propagated through the cluster state before finishing the indexing.

In some of our tests we rely on mappings being available as soon as they've been
introduced in a document that indexed correctly. By removing the immediate retry
we always wait for this to be the case.

Resolves elastic#38428
Supercedes elastic#38579
Relates to elastic#38711
@dakrone dakrone added :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v7.0.0 v8.0.0 v7.2.0 labels Feb 13, 2019
@dakrone dakrone requested a review from bleskes February 13, 2019 23:10
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Feb 13, 2019

Pinging @elastic/es-distributed

@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Feb 13, 2019 •
edited
Loading

@bleskes this is the issue I talked to you about, there is more details about it on #38579 which solves one failure caused by it by waiting, I think this is a better solution though. Your name comes up with a refactor in #31821 so I thought you might want to take a look at it.

bleskes
bleskes approved these changes Feb 14, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

LGTM. I can't think of any reason why not to do this and it's a good simplification. I'd like @ywelsch to sanity check this too.

PS. This doesn't guarantee that the cluster state is processed on all nodes when an indexing operation returns. You can still have a case where one node (that's not relevant for the indexing operation) didn't process the committed cluster state.

@dakrone dakrone mentioned this pull request Feb 14, 2019
Closed
@dakrone dakrone requested a review from ywelsch February 14, 2019 15:16
ywelsch
ywelsch approved these changes Feb 14, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@ywelsch ywelsch 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 @dakrone

@dakrone dakrone merged commit 2a03802 into elastic:master Feb 14, 2019
dakrone added a commit that referenced this pull request Feb 14, 2019
@dakrone
Remove immediate operation retry after mapping update (#38873)
0c733c0 
Prior to this commit, when an indexing operation resulted in an
`Engine.Result.Type.MAPPING_UPDATE_REQUIRED`, TransportShardBulkAction
immediately retries the indexing operation to see if it succeeds. In the event
that it succeeds the context does not wait until the mapping update has
propagated through the cluster state before finishing the indexing.

In some of our tests we rely on mappings being available as soon as they've been
introduced in a document that indexed correctly. By removing the immediate retry
we always wait for this to be the case.

Resolves #38428
Supercedes #38579
Relates to #38711
dakrone added a commit that referenced this pull request Feb 14, 2019
@dakrone
Remove immediate operation retry after mapping update (#38873)
aff91b5 
Prior to this commit, when an indexing operation resulted in an
`Engine.Result.Type.MAPPING_UPDATE_REQUIRED`, TransportShardBulkAction
immediately retries the indexing operation to see if it succeeds. In the event
that it succeeds the context does not wait until the mapping update has
propagated through the cluster state before finishing the indexing.

In some of our tests we rely on mappings being available as soon as they've been
introduced in a document that indexed correctly. By removing the immediate retry
we always wait for this to be the case.

Resolves #38428
Supercedes #38579
Relates to #38711
@dakrone dakrone added the v6.7.0 label Feb 14, 2019
dakrone added a commit that referenced this pull request Feb 14, 2019
@dakrone
Remove immediate operation retry after mapping update (#38873)
186a41b 
Prior to this commit, when an indexing operation resulted in an
`Engine.Result.Type.MAPPING_UPDATE_REQUIRED`, TransportShardBulkAction
immediately retries the indexing operation to see if it succeeds. In the event
that it succeeds the context does not wait until the mapping update has
propagated through the cluster state before finishing the indexing.

In some of our tests we rely on mappings being available as soon as they've been
introduced in a document that indexed correctly. By removing the immediate retry
we always wait for this to be the case.

Resolves #38428
Supercedes #38579
Relates to #38711
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Feb 14, 2019
@dakrone
Revert "Mute failing test (elastic#38781)"
9785e2e 
This reverts commit b91e058.

This should be fixed by elastic#38873
@dakrone dakrone mentioned this pull request Feb 14, 2019
Merged
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 14, 2019
@jasontedor
Merge remote-tracking branch 'elastic/master' into retention-lease-ccr
b8b2537 
* elastic/master:
  Remove immediate operation retry after mapping update (elastic#38873)
  Remove mentioning of types from bulk API docs (elastic#38896)
  SQL: change JDBC setup URL in the documentation (elastic#38564)
  Skip BWC tests in checkPart1 and checkPart2 (elastic#38730)
  Enable silent FollowersCheckerTest (elastic#38851)
  Update TESTING.asciidoc with platform specific instructions (elastic#38802)
  Use consistent view of realms for authentication (elastic#38815)
  Stabilize RareClusterState (elastic#38671)
  Increase Timeout in UnicastZenPingTests (elastic#38893)
  Do not recommend installing vagrant-winrm elastic#38887
  _cat/indices with Security, hide names when wildcard (elastic#38824)
  SQL: fall back to using the field name for column label (elastic#38842)
  Fix LocalIndexFollowingIT#testRemoveRemoteConnection() test (elastic#38709)
  Remove joda time mentions in documentation (elastic#38720)
  Add enabled status for token and api key service (elastic#38687)
@dakrone dakrone deleted the remove-immediate-index-retry branch February 14, 2019 18:03
pull bot pushed a commit to scher200/elasticsearch that referenced this pull request Feb 19, 2019
@dakrone
Revert "Mute failing test 20_mix_typless_typefull (elastic#38781)" (e…
f9d11ca 
…lastic#38912)

This reverts commit b91e058.

This should be fixed by elastic#38873

Resolves elastic#38711
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Feb 19, 2019
@dakrone
Revert "Mute failing test 20_mix_typless_typefull (elastic#38781)" (e…
aec7a55 
…lastic#38912)

This reverts commit b91e058.

This should be fixed by elastic#38873

Resolves elastic#38711
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Feb 19, 2019
@dakrone
Revert "Mute failing test 20_mix_typless_typefull (elastic#38781)" (e…
d0bc7a2 
…lastic#38912)

This reverts commit b91e058.

This should be fixed by elastic#38873

Resolves elastic#38711
This was referenced Feb 19, 2019
Merged
Merged
dakrone added a commit that referenced this pull request Feb 19, 2019
@dakrone
Revert "Mute failing test 20_mix_typless_typefull (#38781)" (#38912) (#…
41ac6f9 
…39141)

Backport of #38912

This reverts commit b91e058.

This should be fixed by #38873

Resolves #38711
dakrone added a commit that referenced this pull request Feb 19, 2019
@dakrone
Revert "Mute failing test 20_mix_typless_typefull (#38781)" (#38912) (#…
586db5e 
…39142)

Backport of #38912 

This reverts commit b91e058.

This should be fixed by #38873

Resolves #38711
@dakrone dakrone mentioned this pull request Feb 20, 2019
Merged
@danielmitterdorfer danielmitterdorfer added the >test Issues or PRs that are addressing/adding tests label Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@bleskes bleskes bleskes approved these changes

@ywelsch ywelsch ywelsch approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >test Issues or PRs that are addressing/adding tests v6.7.0 v7.0.0-rc2 v7.2.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[CI] Failure in org.elasticsearch.index.mapper.DynamicMappingTests#testMappingVersionAfterDynamicMappingUpdate

6 participants

@dakrone @elasticmachine @bleskes @ywelsch @jakelandis @danielmitterdorfer