From 7d80ea45442e84c25669db61cb7376ba0cd10ba5 Mon Sep 17 00:00:00 2001 From: anand76 Date: Wed, 4 Jun 2025 17:46:56 -0700 Subject: [PATCH] Fix iterator errors for CFs with disallow_memtable_writes (#13663) Summary: Iterator seek returns "SeekAndValidate() not implemented" error if the disallow_memtable_writes CF option is set along with paranoid_memory_checks. The fix is to sanitize the paranoid_memory_checks option to false, which should be safe since the memtable is guaranteed to be empty. Pull Request resolved: https://github.com/facebook/rocksdb/pull/13663 Test Plan: Update unit test in db_basic_test.cc Reviewed By: pdillinger Differential Revision: D75973515 Pulled By: anand1976 fbshipit-source-id: 3f381f19dcda72e3b78ee375f755fb4809c6b99c --- db/db_basic_test.cc | 6 ++++++ memtable/vectorrep.cc | 20 +++++++++++++++++++ ...isallow_memtable_writes_paranoid_checks.md | 1 + 3 files changed, 27 insertions(+) create mode 100644 unreleased_history/bug_fixes/disallow_memtable_writes_paranoid_checks.md diff --git a/db/db_basic_test.cc b/db/db_basic_test.cc index 55323d29de..cb7313e090 100644 --- a/db/db_basic_test.cc +++ b/db/db_basic_test.cc @@ -5086,6 +5086,7 @@ TEST_F(DBBasicTest, DisallowMemtableWrite) { options_allow.create_if_missing = true; Options options_disallow = options_allow; options_disallow.disallow_memtable_writes = true; + options_disallow.paranoid_memory_checks = true; DestroyAndReopen(options_allow); // CFs allowing and disallowing memtable write @@ -5125,6 +5126,11 @@ TEST_F(DBBasicTest, DisallowMemtableWrite) { EXPECT_EQ(Get(2, "b2"), "2"); EXPECT_EQ(Get(3, "b3"), "NOT_FOUND"); + std::unique_ptr iter( + dbfull()->NewIterator(ReadOptions(), handles_[3])); + iter->Seek("a3"); + ASSERT_OK(iter->status()); + iter.reset(); // When the DB is re-opened with WAL entries for a CF that is newly setting // disallow_memtable_writes, we detect that and fail the open gracefully. ASSERT_EQ(TryReopenWithColumnFamilies( diff --git a/memtable/vectorrep.cc b/memtable/vectorrep.cc index 9b0192cb8e..9a50bdc9fb 100644 --- a/memtable/vectorrep.cc +++ b/memtable/vectorrep.cc @@ -79,6 +79,10 @@ class VectorRep : public MemTableRep { // Advance to the first entry with a key >= target void Seek(const Slice& user_key, const char* memtable_key) override; + // Seek and do some memory validation + Status SeekAndValidate(const Slice& internal_key, const char* memtable_key, + bool allow_data_in_errors) override; + // Advance to the first entry with a key <= target void SeekForPrev(const Slice& user_key, const char* memtable_key) override; @@ -221,6 +225,22 @@ void VectorRep::Iterator::Seek(const Slice& user_key, .first; } +Status VectorRep::Iterator::SeekAndValidate(const Slice& /* internal_key */, + const char* /* memtable_key */, + bool /* allow_data_in_errors */) { + if (vrep_) { + WriteLock l(&vrep_->rwlock_); + if (bucket_->begin() == bucket_->end()) { + // Memtable is empty + return Status::OK(); + } else { + return Status::NotSupported("SeekAndValidate() not implemented"); + } + } else { + return Status::NotSupported("SeekAndValidate() not implemented"); + } +} + // Advance to the first entry with a key <= target void VectorRep::Iterator::SeekForPrev(const Slice& /*user_key*/, const char* /*memtable_key*/) { diff --git a/unreleased_history/bug_fixes/disallow_memtable_writes_paranoid_checks.md b/unreleased_history/bug_fixes/disallow_memtable_writes_paranoid_checks.md new file mode 100644 index 0000000000..d4aea98327 --- /dev/null +++ b/unreleased_history/bug_fixes/disallow_memtable_writes_paranoid_checks.md @@ -0,0 +1 @@ +Fix iterator operations returning NotImplemented status if disallow_memtable_writes and paranoid_memory_checks CF options are both set.