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
During relocation, process pending mapping update in phase 2 #6762
Conversation
During phase1 we copy over all lucene segments. These make refer to mapping updates that are still queued up to be sent to master. We must make sure those pending updates are sent before completing the relocation. Relates to elastic#6648
I think that there is a cleaner way to do it, by simply sending an update mapping per type, with a listener, and waiting for the response. This will guarantee that all the mappings have been properly processed by the master node. |
…changes not only pending
@kimchy pushed another commit based on your suggestion |
@@ -251,22 +258,55 @@ public void phase2(Translog.Snapshot snapshot) throws ElasticsearchException { | |||
if (shard.state() == IndexShardState.CLOSED) { | |||
throw new IndexShardClosedException(request.shardId()); | |||
} | |||
logger.trace("[{}][{}] recovery [phase2] to {}: start", request.shardId().index().name(), request.shardId().id(), request.targetNode()); | |||
logger.trace("{} recovery [phase2] to {}: start", request.shardId().index().name(), request.shardId().id(), request.targetNode()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the change to the logging? now we don't properly log the parameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you forgot to change it to request.shardId as a single parameter, instead of the index name and shard id both
left minor comment, LGTM otherwise |
Delete and update mapping execution order has changed with #6762
Delete and update mapping execution order has changed with #6762
The change added in elastic#6762 helps making sure the pending mapping updates are processed on all nodes to prevent moving shards to nodes which are not yet fully aware of the new mapping. However it introduced a racing condition delete_mapping operations, potentially causing a type to be added after it's deletion. This commit solves this by only sending a mapping update if the mapping source has actually changed.
The change added in #6762 helps making sure the pending mapping updates are processed on all nodes to prevent moving shards to nodes which are not yet fully aware of the new mapping. However it introduced a racing condition delete_mapping operations, potentially causing a type to be added after it's deletion. This commit solves this by only sending a mapping update if the mapping source has actually changed. Closes #6772
The change added in #6762 helps making sure the pending mapping updates are processed on all nodes to prevent moving shards to nodes which are not yet fully aware of the new mapping. However it introduced a racing condition delete_mapping operations, potentially causing a type to be added after it's deletion. This commit solves this by only sending a mapping update if the mapping source has actually changed. Closes #6772
During phase1 we copy over all lucene segments. These make refer to mapping updates that are still queued up to be sent to master. We must make sure those pending updates are sent before completing the relocation.
Relates to #6648