diff --git a/db/db_iter.cc b/db/db_iter.cc index b6f771fd1..bfe7c7213 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -811,12 +811,19 @@ bool DBIter::FindValueForCurrentKey() { ikey, RangeDelPositioningMode::kBackwardTraversal)) { last_key_entry_type = kTypeRangeDeletion; PERF_COUNTER_ADD(internal_delete_skipped_count, 1); - } else { - assert(iter_.iter()->IsValuePinned()); + } else if (iter_.iter()->IsValuePinned()) { pinned_value_ = iter_.value(); + } else { + valid_ = false; + status_ = Status::NotSupported( + "Backward iteration not supported if underlying iterator's value " + "cannot be pinned."); } merge_context_.Clear(); last_not_merge_type = last_key_entry_type; + if (!status_.ok()) { + return false; + } break; case kTypeDeletion: case kTypeSingleDeletion: diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index 8d2b0a7c0..12beaa4af 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -3021,6 +3021,44 @@ TEST_F(DBIteratorWithReadCallbackTest, ReadCallback) { delete iter; } +TEST_F(DBIteratorTest, BackwardIterationOnInplaceUpdateMemtable) { + Options options = CurrentOptions(); + options.create_if_missing = true; + options.inplace_update_support = false; + options.env = env_; + DestroyAndReopen(options); + constexpr int kNumKeys = 10; + + // Write kNumKeys to WAL. + for (int i = 0; i < kNumKeys; ++i) { + ASSERT_OK(Put(Key(i), "val")); + } + ReadOptions read_opts; + read_opts.total_order_seek = true; + { + std::unique_ptr iter(db_->NewIterator(read_opts)); + int count = 0; + for (iter->SeekToLast(); iter->Valid(); iter->Prev()) { + ++count; + } + ASSERT_EQ(kNumKeys, count); + } + + // Reopen and rebuild the memtable from WAL. + options.create_if_missing = false; + options.avoid_flush_during_recovery = true; + options.inplace_update_support = true; + options.allow_concurrent_memtable_write = false; + Reopen(options); + { + std::unique_ptr iter(db_->NewIterator(read_opts)); + iter->SeekToLast(); + // Backward iteration not supported due to inplace_update_support = true. + ASSERT_TRUE(iter->status().IsNotSupported()); + ASSERT_FALSE(iter->Valid()); + } +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index c76c60416..bcbd73d3e 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -237,6 +237,7 @@ struct AdvancedColumnFamilyOptions { // achieve point-in-time consistency using snapshot or iterator (assuming // concurrent updates). Hence iterator and multi-get will return results // which are not consistent as of any point-in-time. + // Backward iteration on memtables will not work either. // If inplace_callback function is not set, // Put(key, new_value) will update inplace the existing_value iff // * key exists in current memtable