Fix bugs introduced by D17961
Summary: D17961 has two bugs: (1) two level iterator fails to populate FileMetaData.table_reader, causing performance regression. (2) table cache handle the !status.ok() case in the wrong place, causing seg fault which shouldn't happen. Test Plan: make all check Reviewers: ljin, igor, haobo Reviewed By: ljin CC: yhchiang, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D17991
This commit is contained in:
parent
ce353c2474
commit
651792251a
@ -112,11 +112,11 @@ Iterator* TableCache::NewIterator(const ReadOptions& options,
|
||||
if (table_reader == nullptr) {
|
||||
s = FindTable(toptions, icomparator, file_meta.number, file_meta.file_size,
|
||||
&handle, nullptr, options.read_tier == kBlockCacheTier);
|
||||
if (!s.ok()) {
|
||||
return NewErrorIterator(s);
|
||||
}
|
||||
table_reader = GetTableReaderFromHandle(handle);
|
||||
}
|
||||
if (!s.ok()) {
|
||||
return NewErrorIterator(s);
|
||||
}
|
||||
|
||||
Iterator* result = table_reader->NewIterator(options);
|
||||
if (handle != nullptr) {
|
||||
@ -146,7 +146,9 @@ Status TableCache::Get(const ReadOptions& options,
|
||||
s = FindTable(storage_options_, internal_comparator, file_meta.number,
|
||||
file_meta.file_size, &handle, table_io,
|
||||
options.read_tier == kBlockCacheTier);
|
||||
t = GetTableReaderFromHandle(handle);
|
||||
if (s.ok()) {
|
||||
t = GetTableReaderFromHandle(handle);
|
||||
}
|
||||
}
|
||||
if (s.ok()) {
|
||||
s = t->Get(options, k, arg, saver, mark_key_may_exist);
|
||||
|
@ -151,7 +151,7 @@ namespace {
|
||||
struct EncodedFileMetaData {
|
||||
uint64_t number; // file number
|
||||
uint64_t file_size; // file size
|
||||
Cache::Handle* table_reader_handle; // cached table reader's handler
|
||||
TableReader* table_reader; // cached table reader
|
||||
};
|
||||
} // namespace
|
||||
|
||||
@ -199,7 +199,7 @@ class Version::LevelFileNumIterator : public Iterator {
|
||||
auto* file_meta = (*flist_)[index_];
|
||||
current_value_.number = file_meta->number;
|
||||
current_value_.file_size = file_meta->file_size;
|
||||
current_value_.table_reader_handle = file_meta->table_reader_handle;
|
||||
current_value_.table_reader = file_meta->table_reader;
|
||||
return Slice(reinterpret_cast<const char*>(¤t_value_),
|
||||
sizeof(EncodedFileMetaData));
|
||||
}
|
||||
@ -231,7 +231,7 @@ static Iterator* GetFileIterator(void* arg, const ReadOptions& options,
|
||||
const EncodedFileMetaData* encoded_meta =
|
||||
reinterpret_cast<const EncodedFileMetaData*>(file_value.data());
|
||||
FileMetaData meta(encoded_meta->number, encoded_meta->file_size);
|
||||
meta.table_reader_handle = encoded_meta->table_reader_handle;
|
||||
meta.table_reader = encoded_meta->table_reader;
|
||||
return cache->NewIterator(
|
||||
options.prefix ? options_copy : options, soptions, icomparator, meta,
|
||||
nullptr /* don't need reference to table*/, for_compaction);
|
||||
@ -257,7 +257,7 @@ bool Version::PrefixMayMatch(const ReadOptions& options,
|
||||
reinterpret_cast<const EncodedFileMetaData*>(
|
||||
level_iter->value().data());
|
||||
FileMetaData meta(encoded_meta->number, encoded_meta->file_size);
|
||||
meta.table_reader_handle = encoded_meta->table_reader_handle;
|
||||
meta.table_reader = encoded_meta->table_reader;
|
||||
may_match = cfd_->table_cache()->PrefixMayMatch(
|
||||
options, cfd_->internal_comparator(), meta, internal_prefix, nullptr);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user