From 294124c660cffa6418c0f6a3de7228c38da64350 Mon Sep 17 00:00:00 2001 From: Evan Jones Date: Thu, 18 Jun 2026 14:54:17 -0400 Subject: [PATCH] GetUpdatesSince: Fix assert when polling an empty DB TransactionLogIterator supports polling the end of the WAL e.g. db_log_iter_test.cc test TransactionLogIteratorStallAtLastRecord. To poll, we need to call Next() on an iterator that is at the end (iter->Valid() == false). However, if the iterator is created when there are no WAL files (e.g. an empty DB), the call to Next() hits an assert in NextImpl: assert(current_log_reader_); To fix this, make an iterator without files return a TryAgain status code instead. This should not affect any existing applications, since if they are doing this, they will crash. transaction_log_impl.cc: * TransactionLogIteratorImpl::NextImpl: Check for no files and return TryAgain. * db.h: GetUpdatesSince doc comment: Reference the documentation on TransactionLogIterator. * transaction_log.h: Clarify how to use TransactionLogIterator. db_log_iter_test: * OpenTransactionLogIter: Remove EXPECT_TRUE(iter->Valid()): this is not true if the database is empty. * TransactionLogIteratorPollNoWALFiles: Test polling the iterator in two cases: the database is empty, and all WAL files were purged. These previously triggered an assertion in Next(). --- db/db_log_iter_test.cc | 67 ++++++++++++++++++++++++++++++- db/transaction_log_impl.cc | 17 +++++++- include/rocksdb/db.h | 4 +- include/rocksdb/transaction_log.h | 28 +++++++++---- 4 files changed, 104 insertions(+), 12 deletions(-) diff --git a/db/db_log_iter_test.cc b/db/db_log_iter_test.cc index 62b1f893d5c2..8e6e0aec1d97 100644 --- a/db/db_log_iter_test.cc +++ b/db/db_log_iter_test.cc @@ -28,7 +28,6 @@ class DBTestXactLogIterator : public DBTestBase { std::unique_ptr iter; Status status = dbfull()->GetUpdatesSince(seq, &iter); EXPECT_OK(status); - EXPECT_TRUE(iter->Valid()); return iter; } }; @@ -210,6 +209,72 @@ TEST_F(DBTestXactLogIterator, TransactionLogIteratorStallAtLastRecord) { } while (ChangeCompactOptions()); } +TEST_F(DBTestXactLogIterator, TransactionLogIteratorPollNoWALFiles) { + do { + // read the WAL on an empty DB: no records + auto iter = OpenTransactionLogIter(0); + ASSERT_OK(iter->status()); + ASSERT_FALSE(iter->Valid()); + + // poll for WAL records when there is no WAL: used to crash now returns + // TryAgain + iter->Next(); + ASSERT_EQ(Status::kTryAgain, iter->status().code()); + ASSERT_FALSE(iter->Valid()); + iter->Next(); + ASSERT_EQ(Status::kTryAgain, iter->status().code()); + ASSERT_FALSE(iter->Valid()); + + // put a record: iterator still returns TryAgain + ASSERT_OK(Put("key1", DummyString(1024))); + iter->Next(); + ASSERT_EQ(Status::kTryAgain, iter->status().code()); + ASSERT_FALSE(iter->Valid()); + + // open the iterator: positioned at batch with key1 + iter = OpenTransactionLogIter(0); + ASSERT_OK(iter->status()); + ASSERT_TRUE(iter->Valid()); + BatchResult batch = iter->GetBatch(); + ASSERT_EQ(1, batch.sequence); + ASSERT_EQ(1, batch.writeBatchPtr->Count()); + + // flush the memtable and WaitForCompact to wait for the WAL to be removed + Status status = dbfull()->Flush(FlushOptions()); + ASSERT_OK(status); + WaitForCompactOptions wait_options; + wait_options.wait_for_purge = true; + status = dbfull()->WaitForCompact(wait_options); + ASSERT_OK(status); + + // open a new iterator: no WAL records + auto iter2 = OpenTransactionLogIter(0); + ASSERT_OK(iter2->status()); + ASSERT_FALSE(iter2->Valid()); + + // put another key: creates a new WAL + ASSERT_OK(Put("key2", DummyString(1024))); + + // iter2 was created with no WAL files: Next returns TryAgain + iter2->Next(); + ASSERT_EQ(Status::kTryAgain, iter2->status().code()); + ASSERT_FALSE(iter2->Valid()); + + // iter: Next also returns TryAgain because there are new wal files + iter->Next(); + ASSERT_EQ(Status::kTryAgain, iter->status().code()); + ASSERT_FALSE(iter->Valid()); + + // reopening the iterator returns the second write + iter = OpenTransactionLogIter(0); + ASSERT_OK(iter->status()); + ASSERT_TRUE(iter->Valid()); + batch = iter->GetBatch(); + ASSERT_EQ(2, batch.sequence); + ASSERT_EQ(1, batch.writeBatchPtr->Count()); + } while (ChangeCompactOptions()); +} + TEST_F(DBTestXactLogIterator, TransactionLogIteratorCheckAfterRestart) { do { Options options = OptionsForLogIterTest(); diff --git a/db/transaction_log_impl.cc b/db/transaction_log_impl.cc index 7c8e20435e2d..7825fa02ea68 100644 --- a/db/transaction_log_impl.cc +++ b/db/transaction_log_impl.cc @@ -173,10 +173,24 @@ void TransactionLogIteratorImpl::Next() { return NextImpl(false); } +// Returns a Status with code TryAgain and a message to re-create the iterator. +static Status TryAgainStatus() { + static const char* const msg = "Create a new iterator to fetch the new tail."; + return Status::TryAgain(msg); +} + void TransactionLogIteratorImpl::NextImpl(bool internal) { Slice record; is_valid_ = false; if (!internal && !started_) { + // if the iterator has no files and we are polling for WAL updates: we can't + // tell if the state has changed. Return TryAgain. + if (files_->empty()) { + current_status_ = TryAgainStatus(); + assert(!Valid()); + return; + } + // Runs every time until we can seek to the start sequence SeekToStartSequence(); } @@ -217,8 +231,7 @@ void TransactionLogIteratorImpl::NextImpl(bool internal) { if (current_last_seq_ == versions_->LastSequence()) { current_status_ = Status::OK(); } else { - const char* msg = "Create a new iterator to fetch the new tail."; - current_status_ = Status::TryAgain(msg); + current_status_ = TryAgainStatus(); } return; } diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index 456109f568fc..4b9807a82299 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -1920,7 +1920,9 @@ class DB { // write-batch exists, then iter is positioned at the next write-batch whose // start_seq > seq_number. // - // Returns Status::OK if iterator is valid + // Returns Status::OK if iterator is valid. See the comment on + // TransactionLogIterator for how to use the iterator. + // // Must set WAL_ttl_seconds or WAL_size_limit_MB to large values to // use this api, else the WAL files will get // cleared aggressively and the iterator might keep getting invalid before diff --git a/include/rocksdb/transaction_log.h b/include/rocksdb/transaction_log.h index d3e335748634..4e3d3e146b65 100644 --- a/include/rocksdb/transaction_log.h +++ b/include/rocksdb/transaction_log.h @@ -86,29 +86,41 @@ struct BatchResult { } }; -// A TransactionLogIterator is used to iterate over the transactions in a db. +// TransactionLogIterator iterates over WriteBatches in the write-ahead log. // One run of the iterator is continuous, i.e. the iterator will stop at the -// beginning of any gap in sequences +// beginning of any gap in sequences. It can also be efficiently polled for +// new WAL records. To use it: +// +// 1. Create the iterator. +// 2. While Valid() == true: call GetBatch(), then call Next() to advance. +// 3. If Valid() is false: check status(). If status() is OK: the iterator +// can be polled for new WAL records by calling Next() then Valid(). +// +// The iterator will return Valid() false and status() TryAgain in two cases: +// 1. A new WAL file was added after the iterator was created. +// 2. The iterator was created when there were no WAL files and Next() was +// called. class TransactionLogIterator { public: TransactionLogIterator() {} virtual ~TransactionLogIterator() {} // An iterator is either positioned at a WriteBatch or not valid. - // This method returns true if the iterator is valid. - // Can read data from a valid iterator. + // This method returns true if the iterator is valid. If it is not valid, + // call status() to check for errors. virtual bool Valid() = 0; - // Moves the iterator to the next WriteBatch. - // REQUIRES: Valid() to be true. + // Moves the iterator to the next WriteBatch. If the iterator is at the end, + // and status() == OK, this can be used to poll for updates. virtual void Next() = 0; // Returns ok if the iterator is valid. // Returns the Error when something has gone wrong. virtual Status status() = 0; - // If valid return's the current write_batch and the sequence number of the - // earliest transaction contained in the batch. + // If valid: returns the current write_batch and the sequence number of the + // earliest transaction contained in the batch. This can only be called once + // since it moves the WriteBatch. // ONLY use if Valid() is true and status() is OK. virtual BatchResult GetBatch() = 0;