andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix bugs introduced by D17961

Browse Source 
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

Conflicts:
	db/version_set.cc
This commit is contained in:
sdong 2014-04-17 15:14:04 -07:00
parent d705755e52
commit 034b494774
2 changed files with 10 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
db/table_cache.cc
View File

@ -117,11 +117,11 @@ Iterator* TableCache::NewIterator(const ReadOptions& options,
if (table_reader == nullptr) { if (table_reader == nullptr) {
s = FindTable(toptions, icomparator, file_meta.number, file_meta.file_size, s = FindTable(toptions, icomparator, file_meta.number, file_meta.file_size,
&handle, nullptr, options.read_tier == kBlockCacheTier); &handle, nullptr, options.read_tier == kBlockCacheTier);
table_reader = GetTableReaderFromHandle(handle);
}
if (!s.ok()) { if (!s.ok()) {
return NewErrorIterator(s); return NewErrorIterator(s);
} }
table_reader = GetTableReaderFromHandle(handle);
}
Iterator* result = table_reader->NewIterator(options); Iterator* result = table_reader->NewIterator(options);
if (handle != nullptr) { if (handle != nullptr) {
@ -151,8 +151,10 @@ Status TableCache::Get(const ReadOptions& options,
s = FindTable(storage_options_, internal_comparator, file_meta.number, s = FindTable(storage_options_, internal_comparator, file_meta.number,
file_meta.file_size, &handle, table_io, file_meta.file_size, &handle, table_io,
options.read_tier == kBlockCacheTier); options.read_tier == kBlockCacheTier);
if (s.ok()) {
t = GetTableReaderFromHandle(handle); t = GetTableReaderFromHandle(handle);
} }
}
if (s.ok()) { if (s.ok()) {
s = t->Get(options, k, arg, saver, mark_key_may_exist); s = t->Get(options, k, arg, saver, mark_key_may_exist);
if (handle != nullptr) { if (handle != nullptr) {

8
db/version_set.cc
View File

@ -148,7 +148,7 @@ namespace {
struct EncodedFileMetaData { struct EncodedFileMetaData {
uint64_t number; // file number uint64_t number; // file number
uint64_t file_size; // file size uint64_t file_size; // file size
Cache::Handle* table_reader_handle; // cached table reader's handler TableReader* table_reader; // cached table reader
}; };
} // namespace } // namespace
@ -196,7 +196,7 @@ class Version::LevelFileNumIterator : public Iterator {
auto* file_meta = (*flist_)[index_]; auto* file_meta = (*flist_)[index_];
current_value_.number = file_meta->number; current_value_.number = file_meta->number;
current_value_.file_size = file_meta->file_size; 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*>(&current_value_), return Slice(reinterpret_cast<const char*>(&current_value_),
sizeof(EncodedFileMetaData)); sizeof(EncodedFileMetaData));
} }
@ -228,7 +228,7 @@ static Iterator* GetFileIterator(void* arg, const ReadOptions& options,
const EncodedFileMetaData* encoded_meta = const EncodedFileMetaData* encoded_meta =
reinterpret_cast<const EncodedFileMetaData*>(file_value.data()); reinterpret_cast<const EncodedFileMetaData*>(file_value.data());
FileMetaData meta(encoded_meta->number, encoded_meta->file_size); 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( return cache->NewIterator(
options.prefix ? options_copy : options, soptions, icomparator, meta, options.prefix ? options_copy : options, soptions, icomparator, meta,
nullptr /* don't need reference to table*/, for_compaction); nullptr /* don't need reference to table*/, for_compaction);
@ -254,7 +254,7 @@ bool Version::PrefixMayMatch(const ReadOptions& options,
reinterpret_cast<const EncodedFileMetaData*>( reinterpret_cast<const EncodedFileMetaData*>(
level_iter->value().data()); level_iter->value().data());
FileMetaData meta(encoded_meta->number, encoded_meta->file_size); 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 = vset_->table_cache_->PrefixMayMatch(options, vset_->icmp_, meta, may_match = vset_->table_cache_->PrefixMayMatch(options, vset_->icmp_, meta,
internal_prefix, nullptr); internal_prefix, nullptr);
} }