From 034b494774c66738b9c21402d95bb6f4f09b3cd5 Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Thu, 17 Apr 2014 15:14:04 -0700
Subject: [PATCH] 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

Conflicts:
	db/version_set.cc
---
 db/table_cache.cc | 10 ++++++----
 db/version_set.cc |  8 ++++----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/db/table_cache.cc b/db/table_cache.cc
index e99e48b30..dc7c60e3f 100644
--- a/db/table_cache.cc
+++ b/db/table_cache.cc
@@ -117,11 +117,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) {
@@ -151,7 +151,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);
diff --git a/db/version_set.cc b/db/version_set.cc
index 4411dde9d..61cdfab21 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -148,7 +148,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
 
@@ -196,7 +196,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*>(&current_value_),
                  sizeof(EncodedFileMetaData));
   }
@@ -228,7 +228,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);
@@ -254,7 +254,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 = vset_->table_cache_->PrefixMayMatch(options, vset_->icmp_, meta,
                                                     internal_prefix, nullptr);
   }