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

Add compaction order check and data block cache check #195

Merged
merged 4 commits into from
Oct 10, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions table/block_based/block_based_table_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -499,11 +499,15 @@ void BlockBasedTableBuilder::Add(const Slice& key, const Slice& value) {
if (!ok()) return;
ValueType value_type = ExtractValueType(key);
if (IsValueType(value_type)) {
#ifndef NDEBUG
if (r->props.num_entries > r->props.num_range_deletions) {
assert(r->internal_comparator.Compare(key, Slice(r->last_key)) > 0);
if (r->internal_comparator.Compare(key, Slice(r->last_key)) <= 0) {
// We were about to insert keys out of order. Abort.
ROCKS_LOG_ERROR(r->ioptions.info_log,
"Out-of-order key insertion into block based table");
r->status = Status::Corruption("Out-of-order key insertion into table");
return;
}
}
#endif // NDEBUG

auto should_flush = r->flush_block_policy->Update(key, value);
if (should_flush) {
Expand Down
69 changes: 50 additions & 19 deletions table/block_based/block_based_table_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1431,7 +1431,7 @@ Status BlockBasedTable::ReadRangeDelBlock(
} else if (found_range_del_block && !range_del_handle.IsNull()) {
ReadOptions read_options;
std::unique_ptr<InternalIterator> iter(NewDataBlockIterator<DataBlockIter>(
read_options, range_del_handle,
read_options, Slice(), range_del_handle,
/*input_iter=*/nullptr, BlockType::kRangeDeletion,
/*get_context=*/nullptr, lookup_context, Status(), prefetch_buffer));
assert(iter != nullptr);
Expand Down Expand Up @@ -1893,8 +1893,8 @@ InternalIteratorBase<IndexValue>* BlockBasedTable::NewIndexIterator(
// If input_iter is not null, update this iter and return it
template <typename TBlockIter>
TBlockIter* BlockBasedTable::NewDataBlockIterator(
const ReadOptions& ro, const BlockHandle& handle, TBlockIter* input_iter,
BlockType block_type, GetContext* get_context,
const ReadOptions& ro, const Slice& index_key, const BlockHandle& handle,
TBlockIter* input_iter, BlockType block_type, GetContext* get_context,
BlockCacheLookupContext* lookup_context, Status s,
FilePrefetchBuffer* prefetch_buffer, bool for_compaction) const {
PERF_TIMER_GUARD(new_table_block_iter_nanos);
Expand Down Expand Up @@ -1985,6 +1985,16 @@ TBlockIter* BlockBasedTable::NewDataBlockIterator(

block.TransferTo(iter);

if (!index_key.empty()) {
iter->SeekToFirst();
if (iter->Valid()) {
if (rep_->internal_comparator.Compare(index_key, iter->key()) < 0) {
iter->Invalidate(
Status::Corruption("first key in block isn't smaller than or equal "
"last key in index"));
}
}
}
return iter;
}

Expand Down Expand Up @@ -2730,7 +2740,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekImpl(
} else {
// Need to use the data block.
if (!same_block) {
InitDataBlock();
if (!InitDataBlock()) {
return;
}
} else {
// When the user does a reseek, the iterate_upper_bound might have
// changed. CheckDataBlockWithinUpperBound() needs to be called
Expand Down Expand Up @@ -2799,8 +2811,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekForPrev(
}
}

InitDataBlock();

if (!InitDataBlock()) {
return;
}
block_iter_.SeekForPrev(target);

FindKeyBackward();
Expand All @@ -2819,7 +2832,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekToLast() {
ResetDataIter();
return;
}
InitDataBlock();
if (!InitDataBlock()) {
return;
}
block_iter_.SeekToLast();
FindKeyBackward();
CheckDataBlockWithinUpperBound();
Expand Down Expand Up @@ -2858,7 +2873,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::Prev() {
return;
}

InitDataBlock();
if (!InitDataBlock()) {
return;
}
block_iter_.SeekToLast();
} else {
assert(block_iter_points_to_real_block_);
Expand All @@ -2876,7 +2893,7 @@ const size_t
256 * 1024;

template <class TBlockIter, typename TValue>
void BlockBasedTableIterator<TBlockIter, TValue>::InitDataBlock() {
bool BlockBasedTableIterator<TBlockIter, TValue>::InitDataBlock() {
BlockHandle data_block_handle = index_iter_->value().handle;
if (!block_iter_points_to_real_block_ ||
data_block_handle.offset() != prev_block_offset_ ||
Expand Down Expand Up @@ -2937,13 +2954,18 @@ void BlockBasedTableIterator<TBlockIter, TValue>::InitDataBlock() {

Status s;
table_->NewDataBlockIterator<TBlockIter>(
read_options_, data_block_handle, &block_iter_, block_type_,
read_options_, index_iter_->key(), data_block_handle, &block_iter_,
block_type_,
/*get_context=*/nullptr, &lookup_context_, s, prefetch_buffer_.get(),
/*for_compaction=*/lookup_context_.caller ==
TableReaderCaller::kCompaction);
if (!block_iter_.Valid()) {
return false;
}
block_iter_points_to_real_block_ = true;
CheckDataBlockWithinUpperBound();
}
return true;
}

template <class TBlockIter, typename TValue>
Expand All @@ -2953,7 +2975,9 @@ bool BlockBasedTableIterator<TBlockIter, TValue>::MaterializeCurrentBlock() {
assert(index_iter_->Valid());

is_at_first_key_from_index_ = false;
InitDataBlock();
if (!InitDataBlock()) {
return false;
}
assert(block_iter_points_to_real_block_);
block_iter_.SeekToFirst();

Expand Down Expand Up @@ -3030,7 +3054,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::FindBlockForward() {
return;
}

InitDataBlock();
if (!InitDataBlock()) {
return;
}
block_iter_.SeekToFirst();
} while (!block_iter_.Valid());
}
Expand All @@ -3046,7 +3072,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::FindKeyBackward() {
index_iter_->Prev();

if (index_iter_->Valid()) {
InitDataBlock();
if (!InitDataBlock()) {
return;
}
block_iter_.SeekToLast();
} else {
return;
Expand Down Expand Up @@ -3269,8 +3297,8 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key,
DataBlockIter biter;
uint64_t referenced_data_size = 0;
NewDataBlockIterator<DataBlockIter>(
read_options, v.handle, &biter, BlockType::kData, get_context,
&lookup_data_block_context,
read_options, iiter->key(), v.handle, &biter, BlockType::kData,
get_context, &lookup_data_block_context,
/*s=*/Status(), /*prefetch_buffer*/ nullptr);

if (no_io && biter.status().IsIncomplete()) {
Expand Down Expand Up @@ -3550,7 +3578,7 @@ void BlockBasedTable::MultiGet(const ReadOptions& read_options,

next_biter.Invalidate(Status::OK());
NewDataBlockIterator<DataBlockIter>(
read_options, iiter->value().handle, &next_biter,
read_options, iiter->key(), iiter->value().handle, &next_biter,
BlockType::kData, get_context, &lookup_data_block_context,
Status(), nullptr);
biter = &next_biter;
Expand Down Expand Up @@ -3710,7 +3738,8 @@ Status BlockBasedTable::Prefetch(const Slice* const begin,
DataBlockIter biter;

NewDataBlockIterator<DataBlockIter>(
ReadOptions(), block_handle, &biter, /*type=*/BlockType::kData,
ReadOptions(), iiter->key(), block_handle, &biter,
/*type=*/BlockType::kData,
/*get_context=*/nullptr, &lookup_context, Status(),
/*prefetch_buffer=*/nullptr);

Expand Down Expand Up @@ -4007,7 +4036,8 @@ Status BlockBasedTable::GetKVPairsFromDataBlocks(

std::unique_ptr<InternalIterator> datablock_iter;
datablock_iter.reset(NewDataBlockIterator<DataBlockIter>(
ReadOptions(), blockhandles_iter->value().handle,
ReadOptions(), blockhandles_iter->key(),
blockhandles_iter->value().handle,
/*input_iter=*/nullptr, /*type=*/BlockType::kData,
/*get_context=*/nullptr, /*lookup_context=*/nullptr, Status(),
/*prefetch_buffer=*/nullptr));
Expand Down Expand Up @@ -4253,7 +4283,8 @@ Status BlockBasedTable::DumpDataBlocks(WritableFile* out_file) {

std::unique_ptr<InternalIterator> datablock_iter;
datablock_iter.reset(NewDataBlockIterator<DataBlockIter>(
ReadOptions(), blockhandles_iter->value().handle,
ReadOptions(), blockhandles_iter->key(),
blockhandles_iter->value().handle,
/*input_iter=*/nullptr, /*type=*/BlockType::kData,
/*get_context=*/nullptr, /*lookup_context=*/nullptr, Status(),
/*prefetch_buffer=*/nullptr));
Expand Down
7 changes: 4 additions & 3 deletions table/block_based/block_based_table_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,9 @@ class BlockBasedTable : public TableReader {
// input_iter: if it is not null, update this one and return it as Iterator
template <typename TBlockIter>
TBlockIter* NewDataBlockIterator(
const ReadOptions& ro, const BlockHandle& block_handle,
TBlockIter* input_iter, BlockType block_type, GetContext* get_context,
const ReadOptions& ro, const Slice& index_key,
const BlockHandle& block_handle, TBlockIter* input_iter,
BlockType block_type, GetContext* get_context,
BlockCacheLookupContext* lookup_context, Status s,
FilePrefetchBuffer* prefetch_buffer, bool for_compaction = false) const;

Expand Down Expand Up @@ -774,7 +775,7 @@ class BlockBasedTableIterator : public InternalIteratorBase<TValue> {
// If `target` is null, seek to first.
void SeekImpl(const Slice* target);

void InitDataBlock();
bool InitDataBlock();
bool MaterializeCurrentBlock();
void FindKeyForward();
void FindBlockForward();
Expand Down