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

Simplify dynamic mappings updates. #10720

Merged
merged 1 commit into from Apr 23, 2015

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Apr 22, 2015

While dynamic mappings updates are using the same code path as updates from the
API when applied on a data node since #10593, they were still using a different
code path on the master node. This commit makes dynamic updates processed the
same way as updates from the API, which also seems to do a better way at
acknowledgements (I could not reproduce the ConcurrentDynamicTemplateTests
failure anymore). It also adds more checks, like for instance that indexing on
replicas should not trigger dynamic mapping updates since they should have been
handled on the primary before.

// hand putting the mapping would start the river, which expects
// to find a _meta document
// So we have no choice but to index first and send mappings afterwards
indexService.mapperService().merge(indexRequest.type(), new CompressedString(update.toBytes()), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we assigne indexService.mapperService() to a var? just to remove the chaining :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok will do

@s1monw
Copy link
Contributor

s1monw commented Apr 22, 2015

left a couple of comments

@s1monw
Copy link
Contributor

s1monw commented Apr 23, 2015

LGTM I think we should add the UUID in a second step in a followup issue

While dynamic mappings updates are using the same code path as updates from the
API when applied on a data node since elastic#10593, they were still using a different
code path on the master node. This commit makes dynamic updates processed the
same way as updates from the API, which also seems to do a better way at
acknowledgements (I could not reproduce the ConcurrentDynamicTemplateTests
failure anymore). It also adds more checks, like for instance that indexing on
replicas should not trigger dynamic mapping updates since they should have been
handled on the primary before.

Close elastic#10720
@jpountz jpountz force-pushed the fix/simplify_mapperupdatedaction branch from 9301b56 to c6cdf77 Compare April 23, 2015 09:53
jpountz added a commit that referenced this pull request Apr 23, 2015
Mappings: simplify dynamic mappings updates.

Close #10720
@jpountz jpountz merged commit 803c393 into elastic:master Apr 23, 2015
@clintongormley clintongormley changed the title Mappings: simplify dynamic mappings updates. Simplify dynamic mappings updates. Jun 7, 2015
@clintongormley clintongormley added the :Search/Mapping Index mappings, including merging and defining field types label 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 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants