From 6b2e7a2a01836c8338a4fb11b1790917f72cd080 Mon Sep 17 00:00:00 2001 From: sdong Date: Thu, 20 Mar 2014 17:32:55 -0700 Subject: [PATCH] When Options.max_num_files=-1, non level0 files also by pass table cache Summary: This is the part that was not finished when doing the Options.max_num_files=-1 feature. For iterating non level0 SST files (which was done using two level iterator), table cache is not bypassed. With this patch, the leftover feature is done. Test Plan: make all check; change Options.max_num_files=-1 in one of the tests to cover the codes. Reviewers: haobo, igor, dhruba, ljin, yhchiang Reviewed By: haobo CC: leveldb Differential Revision: https://reviews.facebook.net/D17001 --- db/db_test.cc | 1 + db/table_cache.cc | 26 ++++++++++++++++++-------- db/table_cache.h | 2 +- db/version_set.cc | 26 ++++++++++++-------------- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/db/db_test.cc b/db/db_test.cc index d3bbf8382..ef39a014b 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -3330,6 +3330,7 @@ TEST(DBTest, InPlaceUpdateCallbackNoAction) { TEST(DBTest, CompactionFilter) { Options options = CurrentOptions(); + options.max_open_files = -1; options.num_levels = 3; options.max_mem_compaction_level = 0; options.compaction_filter_factory = std::make_shared(); diff --git a/db/table_cache.cc b/db/table_cache.cc index 222e60eda..7058221e0 100644 --- a/db/table_cache.cc +++ b/db/table_cache.cc @@ -192,17 +192,27 @@ Status TableCache::GetTableProperties( bool TableCache::PrefixMayMatch(const ReadOptions& options, const InternalKeyComparator& icomparator, - uint64_t file_number, uint64_t file_size, + const FileMetaData& file_meta, const Slice& internal_prefix, bool* table_io) { - Cache::Handle* handle = nullptr; - Status s = FindTable(storage_options_, icomparator, file_number, file_size, - &handle, table_io); bool may_match = true; - if (s.ok()) { - TableReader* t = GetTableReaderFromHandle(handle); - may_match = t->PrefixMayMatch(internal_prefix); - ReleaseHandle(handle); + auto table_handle = file_meta.table_reader_handle; + if (table_handle == nullptr) { + // Need to get table handle from file number + Status s = FindTable(storage_options_, icomparator, file_meta.number, + file_meta.file_size, &table_handle, table_io); + if (!s.ok()) { + return may_match; + } } + + auto table = GetTableReaderFromHandle(table_handle); + may_match = table->PrefixMayMatch(internal_prefix); + + if (file_meta.table_reader_handle == nullptr) { + // Need to release handle if it is generated from here. + ReleaseHandle(table_handle); + } + return may_match; } diff --git a/db/table_cache.h b/db/table_cache.h index 38f08031d..42dee2f0f 100644 --- a/db/table_cache.h +++ b/db/table_cache.h @@ -58,7 +58,7 @@ class TableCache { // the table index or blooms are not in memory, this may cause an I/O bool PrefixMayMatch(const ReadOptions& options, const InternalKeyComparator& internal_comparator, - uint64_t file_number, uint64_t file_size, + const FileMetaData& file_meta, const Slice& internal_prefix, bool* table_io); // Evict any entry for the specified file number diff --git a/db/version_set.cc b/db/version_set.cc index 3d9b0f128..913263ee0 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -181,18 +181,14 @@ class Version::LevelFileNumIterator : public Iterator { } Slice value() const { assert(Valid()); - EncodeFixed64(value_buf_, (*flist_)[index_]->number); - EncodeFixed64(value_buf_+8, (*flist_)[index_]->file_size); - return Slice(value_buf_, sizeof(value_buf_)); + return Slice(reinterpret_cast((*flist_)[index_]), + sizeof(FileMetaData)); } virtual Status status() const { return Status::OK(); } private: const InternalKeyComparator icmp_; const std::vector* const flist_; uint32_t index_; - - // Backing store for value(). Holds the file number and size. - mutable char value_buf_[16]; }; static Iterator* GetFileIterator(void* arg, const ReadOptions& options, @@ -200,7 +196,7 @@ static Iterator* GetFileIterator(void* arg, const ReadOptions& options, const InternalKeyComparator& icomparator, const Slice& file_value, bool for_compaction) { TableCache* cache = reinterpret_cast(arg); - if (file_value.size() != 16) { + if (file_value.size() != sizeof(FileMetaData)) { return NewErrorIterator( Status::Corruption("FileReader invoked with unexpected value")); } else { @@ -211,11 +207,12 @@ static Iterator* GetFileIterator(void* arg, const ReadOptions& options, options_copy = options; options_copy.prefix = nullptr; } - FileMetaData meta(DecodeFixed64(file_value.data()), - DecodeFixed64(file_value.data() + 8)); + + const FileMetaData* meta_file = + reinterpret_cast(file_value.data()); return cache->NewIterator( - options.prefix ? options_copy : options, soptions, icomparator, meta, - nullptr /* don't need reference to table*/, for_compaction); + options.prefix ? options_copy : options, soptions, icomparator, + *meta_file, nullptr /* don't need reference to table*/, for_compaction); } } @@ -234,10 +231,11 @@ bool Version::PrefixMayMatch(const ReadOptions& options, // key() will always be the biggest value for this SST? may_match = true; } else { + const FileMetaData* meta_file = + reinterpret_cast(level_iter->value().data()); + may_match = vset_->table_cache_->PrefixMayMatch( - options, vset_->icmp_, DecodeFixed64(level_iter->value().data()), - DecodeFixed64(level_iter->value().data() + 8), internal_prefix, - nullptr); + options, vset_->icmp_, *meta_file, internal_prefix, nullptr); } return may_match; }