From 6a6aef25c1f20f5922e1478999fe0e7f59af1712 Mon Sep 17 00:00:00 2001 From: Mike Kolupaev Date: Fri, 10 May 2019 12:36:40 -0700 Subject: [PATCH] Fix crash in BlockBasedTableIterator::Seek() (#5291) Summary: https://github.com/facebook/rocksdb/pull/5256 broke it: `block_iter_.user_key()` may not be valid even if `block_iter_points_to_real_block_` is true. E.g. if there was an IO error or Status::Incomplete. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5291 Differential Revision: D15273324 Pulled By: al13n321 fbshipit-source-id: 442e5b09f9884a58f92a6ac1ca93af719c219886 --- table/block_based_table_reader.cc | 2 +- table/table_test.cc | 38 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index 1dc220dde..576117f0d 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -2325,7 +2325,7 @@ void BlockBasedTableIterator::Seek(const Slice& target) { } bool need_seek_index = true; - if (block_iter_points_to_real_block_) { + if (block_iter_points_to_real_block_ && block_iter_.Valid()) { // Reseek. prev_index_value_ = index_iter_->value(); // We can avoid an index seek if: diff --git a/table/table_test.cc b/table/table_test.cc index a62ce4255..7292ad7c3 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -1798,6 +1798,44 @@ TEST_P(BlockBasedTableTest, PartitionIndexTest) { } } +TEST_P(BlockBasedTableTest, IndexSeekOptimizationIncomplete) { + std::unique_ptr comparator( + new InternalKeyComparator(BytewiseComparator())); + BlockBasedTableOptions table_options = GetBlockBasedTableOptions(); + Options options; + options.table_factory.reset(NewBlockBasedTableFactory(table_options)); + const ImmutableCFOptions ioptions(options); + const MutableCFOptions moptions(options); + + TableConstructor c(BytewiseComparator()); + AddInternalKey(&c, "pika"); + + std::vector keys; + stl_wrappers::KVMap kvmap; + c.Finish(options, ioptions, moptions, table_options, *comparator, &keys, + &kvmap); + ASSERT_EQ(1, keys.size()); + + auto reader = c.GetTableReader(); + ReadOptions ropt; + ropt.read_tier = ReadTier::kBlockCacheTier; + std::unique_ptr iter( + reader->NewIterator(ropt, /* prefix_extractor */ nullptr)); + + auto ikey = [](Slice user_key) { + return InternalKey(user_key, 0, kTypeValue).Encode().ToString(); + }; + + iter->Seek(ikey("pika")); + ASSERT_FALSE(iter->Valid()); + ASSERT_TRUE(iter->status().IsIncomplete()); + + // This used to crash at some point. + iter->Seek(ikey("pika")); + ASSERT_FALSE(iter->Valid()); + ASSERT_TRUE(iter->status().IsIncomplete()); +} + // It's very hard to figure out the index block size of a block accurately. // To make sure we get the index size, we just make sure as key number // grows, the filter block size also grows.