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

Make mapping updates atomic wrt document parsing. #11205

Closed
wants to merge 3 commits into from

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented May 18, 2015

When mapping updates happen concurrently with document parsing, bad things can
happen. For instance, when applying a mapping update we first update the Mapping
object which is used for parsing and then FieldNameAnalyzer which is used by
IndexWriter for analysis. So if you are unlucky, it could happen that a document
was parsed successfully without introducing dynamic updates yet IndexWriter does
not see its analyzer yet.

In order to fix this issue, mapping updates are now protected by a write lock
and document parsing is protected by the read lock associated with this write
lock. This ensures that no documents will be parsed while a mapping update is
being applied, so document parsing will either see none of the update or all of
it.

When mapping updates happen concurrently with document parsing, bad things can
happen. For instance, when applying a mapping update we first update the Mapping
object which is used for parsing and then FieldNameAnalyzer which is used by
IndexWriter for analysis. So if you are unlucky, it could happen that a document
was parsed successfully without introducing dynamic updates yet IndexWriter does
not see its analyzer yet.

In order to fix this issue, mapping updates are now protected by a write lock
and document parsing is protected by the read lock associated with this write
lock. This ensures that no documents will be parsed while a mapping update is
being applied, so document parsing will either see none of the update or all of
it.
@jpountz jpountz added v2.0.0-beta1 :Search/Mapping Index mappings, including merging and defining field types review labels May 18, 2015
@rjernst
Copy link
Member

rjernst commented May 18, 2015

LGTM. Any way to test this?

@rjernst rjernst removed the review label May 18, 2015
@jpountz
Copy link
Contributor Author

jpountz commented May 21, 2015

@s1monw @rjernst We now have a working test, if you comment out the locking on DocumentParser, it fails.

@jpountz
Copy link
Contributor Author

jpountz commented May 21, 2015

Merged manually.

@jpountz jpountz closed this May 21, 2015
@clintongormley clintongormley changed the title Mappings: Make mapping updates atomic wrt document parsing. Make mapping updates atomic wrt document parsing. 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