diff --git a/include/rocksdb/status.h b/include/rocksdb/status.h index b0d7dbef0..ddddee949 100644 --- a/include/rocksdb/status.h +++ b/include/rocksdb/status.h @@ -79,6 +79,7 @@ class Status { KMergeOperandsInsufficientCapacity = 10, kManualCompactionPaused = 11, kOverwritten = 12, + kTxnNotPrepared = 13, kMaxSubCode }; @@ -224,6 +225,13 @@ class Status { return Status(kIOError, kPathNotFound, msg, msg2); } + static Status TxnNotPrepared() { + return Status(kInvalidArgument, kTxnNotPrepared); + } + static Status TxnNotPrepared(const Slice& msg, const Slice& msg2 = Slice()) { + return Status(kInvalidArgument, kTxnNotPrepared, msg, msg2); + } + // Returns true iff the status indicates success. bool ok() const { return code() == kOk; } @@ -315,6 +323,11 @@ class Status { return (code() == kIncomplete) && (subcode() == kManualCompactionPaused); } + // Returns true iff the status indicates a TxnNotPrepared error. + bool IsTxnNotPrepared() const { + return (code() == kInvalidArgument) && (subcode() == kTxnNotPrepared); + } + // Return a string representation of this status suitable for printing. // Returns the string "OK" for success. std::string ToString() const; diff --git a/include/rocksdb/utilities/transaction.h b/include/rocksdb/utilities/transaction.h index 18e27844c..6ebdbcc40 100644 --- a/include/rocksdb/utilities/transaction.h +++ b/include/rocksdb/utilities/transaction.h @@ -139,7 +139,9 @@ class Transaction { // // If this transaction was created by a TransactionDB(), Status::Expired() // may be returned if this transaction has lived for longer than - // TransactionOptions.expiration. + // TransactionOptions.expiration. Status::TxnNotPrepared() may be returned if + // TransactionOptions.skip_prepare is false and Prepare is not called on this + // transaction before Commit. virtual Status Commit() = 0; // Discard all batched writes in this transaction. diff --git a/include/rocksdb/utilities/transaction_db.h b/include/rocksdb/utilities/transaction_db.h index 73b7416a0..8967b7eef 100644 --- a/include/rocksdb/utilities/transaction_db.h +++ b/include/rocksdb/utilities/transaction_db.h @@ -172,6 +172,10 @@ struct TransactionOptions { // Default: false bool skip_concurrency_control = false; + // In pessimistic transaction, if this is true, then you can skip Prepare + // before Commit, otherwise, you must Prepare before Commit. + bool skip_prepare = true; + // See TransactionDBOptions::default_write_batch_flush_threshold for // description. If a negative value is specified, then the default value from // TransactionDBOptions is used. diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc index 722e6321d..a33e5df51 100644 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@ -87,6 +87,7 @@ 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; } PessimisticTransaction::~PessimisticTransaction() { @@ -283,10 +284,11 @@ Status PessimisticTransaction::Commit() { commit_prepared = true; } else if (txn_state_ == STARTED) { // expiration and lock stealing is not a concern - commit_without_prepare = true; - // TODO(myabandeh): what if the user mistakenly forgets prepare? We should - // add an option so that the user explictly express the intention of - // skipping the prepare phase. + if (skip_prepare_) { + commit_without_prepare = true; + } else { + return Status::TxnNotPrepared(); + } } if (commit_without_prepare) { diff --git a/utilities/transactions/pessimistic_transaction.h b/utilities/transactions/pessimistic_transaction.h index 8f2c84405..f81405bc3 100644 --- a/utilities/transactions/pessimistic_transaction.h +++ b/utilities/transactions/pessimistic_transaction.h @@ -120,6 +120,9 @@ class PessimisticTransaction : public TransactionBaseImpl { // Refer to // TransactionOptions::use_only_the_last_commit_time_batch_for_recovery bool use_only_the_last_commit_time_batch_for_recovery_ = false; + // Refer to + // TransactionOptions::skip_prepare + bool skip_prepare_ = false; virtual Status PrepareInternal() = 0; diff --git a/utilities/transactions/transaction_test.cc b/utilities/transactions/transaction_test.cc index 06a88c30c..9668d447c 100644 --- a/utilities/transactions/transaction_test.cc +++ b/utilities/transactions/transaction_test.cc @@ -6205,6 +6205,28 @@ TEST_P(TransactionTest, DoubleCrashInRecovery) { } } +TEST_P(TransactionTest, CommitWithoutPrepare) { + { + // skip_prepare = false. + WriteOptions write_options; + TransactionOptions txn_options; + txn_options.skip_prepare = false; + Transaction* txn = db->BeginTransaction(write_options, txn_options); + ASSERT_TRUE(txn->Commit().IsTxnNotPrepared()); + delete txn; + } + + { + // skip_prepare = true. + WriteOptions write_options; + TransactionOptions txn_options; + txn_options.skip_prepare = true; + Transaction* txn = db->BeginTransaction(write_options, txn_options); + ASSERT_OK(txn->Commit()); + delete txn; + } +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) {