Skip to content

Commit

Permalink
Assert keys/values pinned by range deletion meta-block iterators
Browse files Browse the repository at this point in the history
Summary:
`RangeDelAggregator` holds the pointers returned by `BlockIter::key()` and `BlockIter::value()` so requires the data to which they point is pinned. `BlockIter::key()` points into block memory and is guaranteed to be pinned if and only if prefix encoding is disabled (or, equivalently, restart interval is set to one). I think `BlockIter::value()` is always pinned. Added an assert for these and removed the wrong TODO about increasing restart interval, which would enable key prefix encoding and break the assertion.
Closes #3875

Differential Revision: D8063667

Pulled By: ajkr

fbshipit-source-id: 60b5ebcc0cdd610dd6aad9e74a23378793672c41
  • Loading branch information
ajkr authored and facebook-github-bot committed May 21, 2018
1 parent e410501 commit 7b65521
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 1 deletion.
4 changes: 4 additions & 0 deletions db/range_del_aggregator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ Status RangeDelAggregator::AddTombstones(
input->SeekToFirst();
bool first_iter = true;
while (input->Valid()) {
// The tombstone map holds slices into the iterator's memory. This assert
// ensures pinning the iterator also pins the keys/values.
assert(input->IsKeyPinned() && input->IsValuePinned());

if (first_iter) {
if (rep_ == nullptr) {
InitRep({upper_bound_});
Expand Down
2 changes: 1 addition & 1 deletion table/block_based_table_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ struct BlockBasedTableBuilder::Rep {
: 0),
data_block(table_options.block_restart_interval,
table_options.use_delta_encoding),
range_del_block(1), // TODO(andrewkr): restart_interval unnecessary
range_del_block(1 /* block_restart_interval */),
internal_prefix_transform(_ioptions.prefix_extractor),
compression_type(_compression_type),
compression_opts(_compression_opts),
Expand Down
3 changes: 3 additions & 0 deletions util/testutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ class VectorIterator : public InternalIterator {

virtual Status status() const override { return Status::OK(); }

virtual bool IsKeyPinned() const override { return true; }
virtual bool IsValuePinned() const override { return true; }

private:
std::vector<std::string> keys_;
std::vector<std::string> values_;
Expand Down

0 comments on commit 7b65521

Please sign in to comment.