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

Wait for mapping updates during local recovery #6666

Closed
wants to merge 2 commits into from

Conversation

kimchy
Copy link
Member

@kimchy kimchy commented Jul 1, 2014

when the primary shard is recovering its translog, make sure to wait for new mapping introductions till the mappings have been updated on the master before finalizing the recovery itself
also, this change performs the mapping updates in a more optimized manner by batching the types to change into a single set and sending after the translog has been replayed

when the primary shard is recovering its translog, make sure to wait for new mapping introductions till the mappings have been updated on the master before finalizing the recovery itself
also, this change performs the mapping updates in a more optimized manner by batching the types to change into a single set and sending after the translog has been replayed
closes elastic#6666
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these classes be static?

Copy link
Member Author

Choose a reason for hiding this comment

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

they can't at their current placement, because they are in non static inner class. they can be moved to outside, and be made static, but then, at least for the value class, we will need to pass the logger as well. I like it like this, feels more encapsulated, and the perf overhead of non static classes should be insignificant in the context of this operation (mapping updates)

@jpountz
Copy link
Contributor

jpountz commented Jul 1, 2014

Just left a question. Otherwise it looks good to me but I'm not familiar with this code so I might have missed obvious bugs.

@uboness
Copy link
Contributor

uboness commented Jul 1, 2014

LGTM... would be nice to have the option for LocalIndexShardGateway:277 to update the mappings in a single batch (so a single request)

@kimchy
Copy link
Member Author

kimchy commented Jul 1, 2014

@uboness yea, that would be nice, but it would be a bigger change, will push this for now

@uboness
Copy link
Contributor

uboness commented Jul 1, 2014

sure

@kimchy kimchy closed this in 2b1823c Jul 1, 2014
kimchy added a commit that referenced this pull request Jul 1, 2014
when the primary shard is recovering its translog, make sure to wait for new mapping introductions till the mappings have been updated on the master before finalizing the recovery itself
also, this change performs the mapping updates in a more optimized manner by batching the types to change into a single set and sending after the translog has been replayed

also, remove the wait for mapping on master in the local state tests since this new behavior covers it

closes #6666

remove waiting for mapping on master since we do it in recovery
@jpountz jpountz removed the review label Jul 16, 2014
@clintongormley clintongormley changed the title wait for mapping updates during local recovery Mapping: Wait for mapping updates during local recovery Jul 16, 2014
@kimchy kimchy deleted the wait_for_mapping_recovery branch August 18, 2014 22:18
@clintongormley clintongormley added the :Search/Mapping Index mappings, including merging and defining field types label Jun 7, 2015
@clintongormley clintongormley changed the title Mapping: Wait for mapping updates during local recovery Wait for mapping updates during local recovery Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Mapping Index mappings, including merging and defining field types v1.3.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants