From 72d1e258cdbdeba4fbd0de116b277959f5bad8bb Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Tue, 2 Mar 2021 22:38:52 -0800 Subject: [PATCH] Possibly bump NUMBER_OF_RESEEKS_IN_ITERATION (#8015) Summary: When changing db iterator direction, we may perform a reseek. Therefore, we should bump the NUMBER_OF_RESEEKS_IN_ITERATION counter. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8015 Test Plan: make check Reviewed By: ltamasi Differential Revision: D26755415 Pulled By: riversand963 fbshipit-source-id: 211f51f1a454bcda768fc46c0dce51edeb7f05fe --- db/db_iter.cc | 2 ++ db/db_iterator_test.cc | 45 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/db/db_iter.cc b/db/db_iter.cc index f25fd8b2c..853e800ec 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -681,6 +681,7 @@ bool DBIter::ReverseToForward() { last_key.SetInternalKey(ParsedInternalKey( saved_key_.GetUserKey(), kMaxSequenceNumber, kValueTypeForSeek)); iter_.Seek(last_key.GetInternalKey()); + RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION); } direction_ = kForward; @@ -731,6 +732,7 @@ bool DBIter::ReverseToBackward() { iter_.SeekToLast(); } } + RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION); } direction_ = kReverse; diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index 2373963e5..a115094c3 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -18,6 +18,7 @@ #include "rocksdb/perf_context.h" #include "table/block_based/flush_block_policy.h" #include "util/random.h" +#include "utilities/merge_operators/string_append/stringappend2.h" namespace ROCKSDB_NAMESPACE { @@ -619,6 +620,40 @@ TEST_P(DBIteratorTest, IterReseek) { delete iter; } +TEST_F(DBIteratorTest, ReseekUponDirectionChange) { + Options options = GetDefaultOptions(); + options.create_if_missing = true; + options.prefix_extractor.reset(NewFixedPrefixTransform(1)); + options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics(); + options.merge_operator.reset( + new StringAppendTESTOperator(/*delim_char=*/' ')); + DestroyAndReopen(options); + ASSERT_OK(Put("foo", "value")); + ASSERT_OK(Put("bar", "value")); + { + std::unique_ptr it(db_->NewIterator(ReadOptions())); + it->SeekToLast(); + it->Prev(); + it->Next(); + } + ASSERT_EQ(1, + options.statistics->getTickerCount(NUMBER_OF_RESEEKS_IN_ITERATION)); + + const std::string merge_key("good"); + ASSERT_OK(Put(merge_key, "orig")); + ASSERT_OK(Merge(merge_key, "suffix")); + { + std::unique_ptr it(db_->NewIterator(ReadOptions())); + it->Seek(merge_key); + ASSERT_TRUE(it->Valid()); + const uint64_t prev_reseek_count = + options.statistics->getTickerCount(NUMBER_OF_RESEEKS_IN_ITERATION); + it->Prev(); + ASSERT_EQ(prev_reseek_count + 1, options.statistics->getTickerCount( + NUMBER_OF_RESEEKS_IN_ITERATION)); + } +} + TEST_P(DBIteratorTest, IterSmallAndLargeMix) { do { CreateAndReopenWithCF({"pikachu"}, CurrentOptions()); @@ -2997,18 +3032,18 @@ TEST_P(DBIteratorTest, Blob) { iter->Prev(); ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 5); iter->Next(); - ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 5); + ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 6); ASSERT_EQ(IterStatus(iter), "b->vb3"); // Switch from forward to reverse iter->SeekToFirst(); - ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 5); - iter->Next(); - ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 5); + ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 6); iter->Next(); ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 6); - iter->Prev(); + iter->Next(); ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 7); + iter->Prev(); + ASSERT_EQ(TestGetTickerCount(options, NUMBER_OF_RESEEKS_IN_ITERATION), 8); ASSERT_EQ(IterStatus(iter), "b->vb3"); }