Skip to content

Commit

Permalink
Add compaction order check and data block cache check (#195)
Browse files Browse the repository at this point in the history
* add compaction order check and data block cache check

Signed-off-by: Connor1996 <[email protected]>
  • Loading branch information
Connor1996 authored Oct 10, 2020
1 parent cb1a562 commit 953f8b2
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 30 deletions.
16 changes: 11 additions & 5 deletions db/version_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,19 @@ class VersionBuilder::Rep {
// Make sure there is no overlap in levels > 0
if (vstorage->InternalComparator()->Compare(f1->largest,
f2->smallest) >= 0) {
fprintf(stderr, "L%d have overlapping ranges %s vs. %s\n", level,
(f1->largest).DebugString(true).c_str(),
(f2->smallest).DebugString(true).c_str());
fprintf(
stderr,
"L%d have overlapping ranges %s of file %s vs. %s of file %s\n",
level, (f1->largest).DebugString(true).c_str(),
NumberToString(f1->fd.GetNumber()).c_str(),
(f2->smallest).DebugString(true).c_str(),
NumberToString(f2->fd.GetNumber()).c_str());
return Status::Corruption(
"L" + NumberToString(level) + " have overlapping ranges " +
(f1->largest).DebugString(true) + " vs. " +
(f2->smallest).DebugString(true));
(f1->largest).DebugString(true) + " of file " +
NumberToString(f1->fd.GetNumber()) + " vs. " +
(f2->smallest).DebugString(true) + " of file " +
NumberToString(f2->fd.GetNumber()));
}
}
}
Expand Down
12 changes: 9 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,17 @@ 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 %s",
rep_->file->file_name().c_str());
r->status = Status::Corruption(
"Out-of-order key insertion into table " + rep_->file->file_name());
return;
}
}
#endif // NDEBUG

auto should_flush = r->flush_block_policy->Update(key, value);
if (should_flush) {
Expand Down
87 changes: 68 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,34 @@ TBlockIter* BlockBasedTable::NewDataBlockIterator(

block.TransferTo(iter);

if (!index_key.empty()) {
iter->SeekToFirst();
if (iter->Valid()) {
auto& comparator = rep_->internal_comparator;
auto user_comparator = comparator.user_comparator();
// index key may not include seq, so check property to decide whether to
// only compare user_key part or not
const bool is_user_key =
rep_->table_properties->index_key_is_user_key > 0;
if ((!is_user_key && comparator.Compare(index_key, iter->key()) < 0) ||
(is_user_key &&
user_comparator->Compare(index_key, iter->user_key()) < 0)) {
ROCKS_LOG_ERROR(rep_->ioptions.info_log,
"first key %s in block[%" PRIu64 ", %" PRIu64
"] of %s isn't smaller than or equal to index key %s\n",
iter->key().ToString(true).c_str(), handle.offset(),
handle.size(), rep_->file->file_name().c_str(),
index_key.ToString(true).c_str());
iter->Invalidate(Status::Corruption(
"first key " + iter->key().ToString(true) + " in block[" +
ToString(handle.offset()) + "," + ToString(handle.size()) +
"] of " + rep_->file->file_name() +
" isn't smaller than or equal to"
"index key " +
index_key.ToString(true)));
}
}
}
return iter;
}

Expand Down Expand Up @@ -2730,7 +2758,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 +2829,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekForPrev(
}
}

InitDataBlock();

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

FindKeyBackward();
Expand All @@ -2819,7 +2850,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekToLast() {
ResetDataIter();
return;
}
InitDataBlock();
if (!InitDataBlock()) {
return;
}
block_iter_.SeekToLast();
FindKeyBackward();
CheckDataBlockWithinUpperBound();
Expand Down Expand Up @@ -2858,7 +2891,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 +2911,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 +2972,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);
block_iter_points_to_real_block_ = true;
if (!block_iter_.status().ok()) {
return false;
}
CheckDataBlockWithinUpperBound();
}
return true;
}

template <class TBlockIter, typename TValue>
Expand All @@ -2953,7 +2993,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 +3072,9 @@ void BlockBasedTableIterator<TBlockIter, TValue>::FindBlockForward() {
return;
}

InitDataBlock();
if (!InitDataBlock()) {
return;
}
block_iter_.SeekToFirst();
} while (!block_iter_.Valid());
}
Expand All @@ -3046,7 +3090,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 +3315,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 +3596,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 +3756,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 +4054,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 +4301,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

0 comments on commit 953f8b2

Please sign in to comment.