Fix corruption in store reader iterator, take 2 #1088
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ah I should have covered this with the unit test. Thanks for the bugfix! |
|
Published as a hotfix on 0.15.2. |
|
@fulmicoton Thanks for the release! No problem, this made me understand a bit more the internals of Tantivy. It's an awesome lib and it's core to my personal project :) |
|
@appaquet Thanks to you! Bugfix on fresh release with an analysis, a patch, and way to reproduce the problem are really my favorite kind of contributions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow up from my previous bug fixed in here. The fix resolved the different panics, but didn't solve the underlying issue when a deleted document is at the beginning of a checkpoint. What happened now is that if a deleted document was at the beginning of the block, we weren't taking it into consideration when skipping, causing the iterator to return deleted documents, and the merge to include deleted documents.
Finding the bug was quite a challenge. At first I though it was another underlying issue in 0.15, but bisecting the commits pointed me to my previous fix 馃槵 You can see how I was consistently reproducing it with gist, which is quite intensive. Running it a few times will eventually lead to corrupted results (searching for a term, but getting results that aren't for the term).
I tweaked the unit tests @PSeitz created to fail when a deleted document is returned by the iterator.