From 832fd644fceab715d4a522ca52ad6121b8f4366f Mon Sep 17 00:00:00 2001 From: Yanqin Jin Date: Wed, 14 Sep 2022 18:28:21 -0700 Subject: [PATCH] Reset pessimistic transaction's read/commit timestamps during Initialize() (#10677) Summary: RocksDB allows reusing old `Transaction` objects when creating new ones. Therefore, we need to reset the transaction's read and commit timestamps back to default values `kMaxTxnTimestamp`. Otherwise, `CommitAndTryCreateSnapshot()` may fail with "Status::InvalidArgument("Different commit ts specified")". Pull Request resolved: https://github.com/facebook/rocksdb/pull/10677 Test Plan: make check Reviewed By: ltamasi Differential Revision: D39513543 Pulled By: riversand963 fbshipit-source-id: bea01cac149bff3a23a2978fc0c3b198243a6291 --- .../transactions/pessimistic_transaction.cc | 3 ++ .../transactions/timestamped_snapshot_test.cc | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc index 6266387a9..cb8fd3bb6 100644 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@ -100,6 +100,9 @@ void PessimisticTransaction::Initialize(const TransactionOptions& txn_options) { use_only_the_last_commit_time_batch_for_recovery_ = txn_options.use_only_the_last_commit_time_batch_for_recovery; skip_prepare_ = txn_options.skip_prepare; + + read_timestamp_ = kMaxTxnTimestamp; + commit_timestamp_ = kMaxTxnTimestamp; } PessimisticTransaction::~PessimisticTransaction() { diff --git a/utilities/transactions/timestamped_snapshot_test.cc b/utilities/transactions/timestamped_snapshot_test.cc index 63e53a6b7..546a77f8b 100644 --- a/utilities/transactions/timestamped_snapshot_test.cc +++ b/utilities/transactions/timestamped_snapshot_test.cc @@ -115,6 +115,45 @@ TEST_P(TransactionTest, WithoutCommitTs) { ASSERT_TRUE(s.IsInvalidArgument()); } +TEST_P(TransactionTest, ReuseExistingTxn) { + Transaction* txn = db->BeginTransaction(WriteOptions(), TransactionOptions()); + assert(txn); + ASSERT_OK(txn->SetName("txn0")); + ASSERT_OK(txn->Put("a", "v1")); + ASSERT_OK(txn->Prepare()); + + auto notifier = std::make_shared(); + std::shared_ptr snapshot1; + Status s = + txn->CommitAndTryCreateSnapshot(notifier, /*commit_ts=*/100, &snapshot1); + ASSERT_OK(s); + ASSERT_EQ(100, snapshot1->GetTimestamp()); + + Transaction* txn1 = + db->BeginTransaction(WriteOptions(), TransactionOptions(), txn); + assert(txn1 == txn); + ASSERT_OK(txn1->SetName("txn1")); + ASSERT_OK(txn->Put("a", "v2")); + ASSERT_OK(txn->Prepare()); + std::shared_ptr snapshot2; + s = txn->CommitAndTryCreateSnapshot(notifier, /*commit_ts=*/110, &snapshot2); + ASSERT_OK(s); + ASSERT_EQ(110, snapshot2->GetTimestamp()); + delete txn; + + { + std::string value; + ReadOptions read_opts; + read_opts.snapshot = snapshot1.get(); + ASSERT_OK(db->Get(read_opts, "a", &value)); + ASSERT_EQ("v1", value); + + read_opts.snapshot = snapshot2.get(); + ASSERT_OK(db->Get(read_opts, "a", &value)); + ASSERT_EQ("v2", value); + } +} + TEST_P(TransactionTest, CreateSnapshotWhenCommit) { std::unique_ptr txn( db->BeginTransaction(WriteOptions(), TransactionOptions()));