Forward port the various DeleteRange PRs #24
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
Summary: When `MockTimeEnv` is used in test to mock time methods, we cannot use `CondVar::TimedWait` because it is using real time, not the mocked time for wait timeout. On Mac the method can return immediately without awaking other waiting threads, if the real time is larger than `wait_until` (which is a mocked time). When that happen, the `wait()` method will fall into an infinite loop. Pull Request resolved: facebook#4560 Differential Revision: D10472851 Pulled By: yiwu-arbug fbshipit-source-id: 898902546ace7db7ac509337dd8677a527209d19
BlockBasedTableIterator now maintains a current tombstone which is a range of keys that are either all deleted in the sstable or all non-deleted. It uses this key range to quickly skip over keys during iteration. The check for non-deleted is pessimistic: a key lying within a non-deleted tombstone range might be deleted by a more precise check of the key's sequence number which is taken care of by the normal RangeDelAggregator::ShouldDelete check in DBIter.
TableCache::NewIterator was failing to pass the file metadata to BlockBasedTableReader::NewIterator, so all of the fancy code in BlockBasedTableIterator to skip ranges of deleted keys was not being used. Changed BlockBasedTableIterator to represent range tombstone endpoints that extend to infinity using a nullptr rather than an empty key. An empty key is a valid key and doing anything with a key other than using it for comparison using the user comparator is invalid.
CollapsedRangeDelMap::GetTombstone would previously return a *Slice that pointed to a Slice within the map. This was dangerous, as a future call to CollapsedRangeDelMap::AddTombstone could erase that slice. Teach GetTombstone to make a copy of the slice that is safe to return.
Adjust the wording of the PartialRangeTombstone description to be more precise about the fact that a nullptr start or end key represents an infinite bound, not a non-existent bound.
Previously, when BlockBasedTableIterator::FindKeyForward seeked the index forward, it failed to call SavePrevIndexValue. This could cause InitDataBlock to fail to notice the updated index position and not load the updated data block, resulting in missing keys.
Change `RangeDelAggregator::GetTombstone,ShouldDeleteRange` to take internal keys instead of user keys.
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.
Also grabs c0f547b to fix a test failure with
RepeatableThread(which doesn't appear to be used anywhere).Fixes #22
This change is