Refactor PessimisticTransaction

Summary:
This patch splits Commit and Prepare into lock-related logic and db-write-related logic. It moves lock-related logic to PessimisticTransaction to be reused by all children classes and movies the existing impl of db-write-related to PrepareInternal, CommitSingleInternal, and CommitInternal in WriteCommittedTxnImpl.
Closes facebook#2691

Differential Revision: D5569464

Pulled By: maysamyabandeh

fbshipit-source-id: d1b8698e69801a4126c7bc211745d05c636f5325

CMakeLists.txt

@@ -518,14 +518,14 @@ set(SOURCES
         utilities/spatialdb/spatial_db.cc
         utilities/table_properties_collectors/compact_on_deletion_collector.cc
         utilities/transactions/optimistic_transaction_db_impl.cc
-        utilities/transactions/optimistic_transaction_impl.cc
+        utilities/transactions/optimistic_transaction.cc
         utilities/transactions/transaction_base.cc
         utilities/transactions/pessimistic_transaction_db.cc
         utilities/transactions/transaction_db_mutex_impl.cc
-        utilities/transactions/transaction_impl.cc
+        utilities/transactions/pessimistic_transaction.cc
         utilities/transactions/transaction_lock_mgr.cc
         utilities/transactions/transaction_util.cc
-        utilities/transactions/write_prepared_transaction_impl.cc
+        utilities/transactions/write_prepared_txn.cc
         utilities/ttl/db_ttl_impl.cc
         utilities/write_batch_with_index/write_batch_with_index.cc
         utilities/write_batch_with_index/write_batch_with_index_internal.cc

TARGETS

@@ -245,14 +245,14 @@ cpp_library(
       "utilities/spatialdb/spatial_db.cc",
       "utilities/table_properties_collectors/compact_on_deletion_collector.cc",
       "utilities/transactions/optimistic_transaction_db_impl.cc",
-      "utilities/transactions/optimistic_transaction_impl.cc",
+      "utilities/transactions/optimistic_transaction.cc",
       "utilities/transactions/transaction_base.cc",
       "utilities/transactions/pessimistic_transaction_db.cc",
       "utilities/transactions/transaction_db_mutex_impl.cc",
-      "utilities/transactions/transaction_impl.cc",
+      "utilities/transactions/pessimistic_transaction.cc",
       "utilities/transactions/transaction_lock_mgr.cc",
       "utilities/transactions/transaction_util.cc",
-      "utilities/transactions/write_prepared_transaction_impl.cc",
+      "utilities/transactions/write_prepared_txn.cc",
       "utilities/ttl/db_ttl_impl.cc",
       "utilities/write_batch_with_index/write_batch_with_index.cc",
       "utilities/write_batch_with_index/write_batch_with_index_internal.cc",

db/db_impl.h

@@ -631,9 +631,9 @@ class DBImpl : public DB {
  private:
   friend class DB;
   friend class InternalStats;
-  friend class PessimisticTxn;
-  friend class WriteCommittedTxnImpl;
-  friend class WritePreparedTxnImpl;
+  friend class PessimisticTransaction;
+  friend class WriteCommittedTxn;
+  friend class WritePreparedTxn;
 #ifndef ROCKSDB_LITE
   friend class ForwardIterator;
 #endif

src.mk

@@ -193,14 +193,14 @@ LIB_SOURCES =                                                   \
   utilities/spatialdb/spatial_db.cc                             \
   utilities/table_properties_collectors/compact_on_deletion_collector.cc \
   utilities/transactions/optimistic_transaction_db_impl.cc      \
-  utilities/transactions/optimistic_transaction_impl.cc         \
+  utilities/transactions/optimistic_transaction.cc         \
   utilities/transactions/transaction_base.cc                    \
   utilities/transactions/pessimistic_transaction_db.cc                 \
   utilities/transactions/transaction_db_mutex_impl.cc           \
-  utilities/transactions/transaction_impl.cc                    \
+  utilities/transactions/pessimistic_transaction.cc                    \
   utilities/transactions/transaction_lock_mgr.cc                \
   utilities/transactions/transaction_util.cc                    \
-  utilities/transactions/write_prepared_transaction_impl.cc     \
+  utilities/transactions/write_prepared_txn.cc     \
   utilities/ttl/db_ttl_impl.cc                                  \
   utilities/write_batch_with_index/write_batch_with_index.cc    \
   utilities/write_batch_with_index/write_batch_with_index_internal.cc    \

utilities/blob_db/blob_db_impl.cc

@@ -32,7 +32,7 @@
 #include "util/random.h"
 #include "util/timer_queue.h"
 #include "utilities/transactions/optimistic_transaction_db_impl.h"
-#include "utilities/transactions/optimistic_transaction_impl.h"
+#include "utilities/transactions/optimistic_transaction.h"
 
 namespace {
 int kBlockBasedTableVersionFormat = 2;

utilities/transactions/optimistic_transaction_impl.cc → utilities/transactions/optimistic_transaction.cc

@@ -5,11 +5,9 @@
 
 #ifndef ROCKSDB_LITE
 
-#include "utilities/transactions/optimistic_transaction_impl.h"
+#include "utilities/transactions/optimistic_transaction.h"
 
-#include <algorithm>
 #include <string>
-#include <vector>
 
 #include "db/column_family.h"
 #include "db/db_impl.h"
@@ -25,40 +23,40 @@ namespace rocksdb {
 
 struct WriteOptions;
 
-OptimisticTransactionImpl::OptimisticTransactionImpl(
+OptimisticTransaction::OptimisticTransaction(
     OptimisticTransactionDB* txn_db, const WriteOptions& write_options,
     const OptimisticTransactionOptions& txn_options)
     : TransactionBaseImpl(txn_db->GetBaseDB(), write_options), txn_db_(txn_db) {
   Initialize(txn_options);
 }
 
-void OptimisticTransactionImpl::Initialize(
+void OptimisticTransaction::Initialize(
     const OptimisticTransactionOptions& txn_options) {
   if (txn_options.set_snapshot) {
     SetSnapshot();
   }
 }
 
-void OptimisticTransactionImpl::Reinitialize(
+void OptimisticTransaction::Reinitialize(
     OptimisticTransactionDB* txn_db, const WriteOptions& write_options,
     const OptimisticTransactionOptions& txn_options) {
   TransactionBaseImpl::Reinitialize(txn_db->GetBaseDB(), write_options);
   Initialize(txn_options);
 }
 
-OptimisticTransactionImpl::~OptimisticTransactionImpl() {
+OptimisticTransaction::~OptimisticTransaction() {
 }
 
-void OptimisticTransactionImpl::Clear() {
+void OptimisticTransaction::Clear() {
   TransactionBaseImpl::Clear();
 }
 
-Status OptimisticTransactionImpl::Prepare() {
+Status OptimisticTransaction::Prepare() {
   return Status::InvalidArgument(
       "Two phase commit not supported for optimistic transactions.");
 }
 
-Status OptimisticTransactionImpl::Commit() {
+Status OptimisticTransaction::Commit() {
   // Set up callback which will call CheckTransactionForConflicts() to
   // check whether this transaction is safe to be committed.
   OptimisticTransactionCallback callback(this);
@@ -75,15 +73,15 @@ Status OptimisticTransactionImpl::Commit() {
   return s;
 }
 
-Status OptimisticTransactionImpl::Rollback() {
+Status OptimisticTransaction::Rollback() {
   Clear();
   return Status::OK();
 }
 
 // Record this key so that we can check it for conflicts at commit time.
 //
 // 'exclusive' is unused for OptimisticTransaction.
-Status OptimisticTransactionImpl::TryLock(ColumnFamilyHandle* column_family,
+Status OptimisticTransaction::TryLock(ColumnFamilyHandle* column_family,
                                           const Slice& key, bool read_only,
                                           bool exclusive, bool untracked) {
   if (untracked) {
@@ -114,7 +112,7 @@ Status OptimisticTransactionImpl::TryLock(ColumnFamilyHandle* column_family,
 //
 // Should only be called on writer thread in order to avoid any race conditions
 // in detecting write conflicts.
-Status OptimisticTransactionImpl::CheckTransactionForConflicts(DB* db) {
+Status OptimisticTransaction::CheckTransactionForConflicts(DB* db) {
   Status result;
 
   auto db_impl = static_cast_with_check<DBImpl, DB>(db);
@@ -127,7 +125,7 @@ Status OptimisticTransactionImpl::CheckTransactionForConflicts(DB* db) {
                                                 true /* cache_only */);
 }
 
-Status OptimisticTransactionImpl::SetName(const TransactionName& name) {
+Status OptimisticTransaction::SetName(const TransactionName& /* unused */) {
   return Status::InvalidArgument("Optimistic transactions cannot be named.");
 }
 

utilities/transactions/optimistic_transaction_impl.h → utilities/transactions/optimistic_transaction.h

@@ -26,13 +26,13 @@
 
 namespace rocksdb {
 
-class OptimisticTransactionImpl : public TransactionBaseImpl {
+class OptimisticTransaction : public TransactionBaseImpl {
  public:
-  OptimisticTransactionImpl(OptimisticTransactionDB* db,
+  OptimisticTransaction(OptimisticTransactionDB* db,
                             const WriteOptions& write_options,
                             const OptimisticTransactionOptions& txn_options);
 
-  virtual ~OptimisticTransactionImpl();
+  virtual ~OptimisticTransaction();
 
   void Reinitialize(OptimisticTransactionDB* txn_db,
                     const WriteOptions& write_options,
@@ -67,20 +67,20 @@ class OptimisticTransactionImpl : public TransactionBaseImpl {
 
   void Clear() override;
 
-  void UnlockGetForUpdate(ColumnFamilyHandle* column_family,
-                          const Slice& key) override {
+  void UnlockGetForUpdate(ColumnFamilyHandle* /* unused */,
+                          const Slice& /* unused */) override {
     // Nothing to unlock.
   }
 
   // No copying allowed
-  OptimisticTransactionImpl(const OptimisticTransactionImpl&);
-  void operator=(const OptimisticTransactionImpl&);
+  OptimisticTransaction(const OptimisticTransaction&);
+  void operator=(const OptimisticTransaction&);
 };
 
 // Used at commit time to trigger transaction validation
 class OptimisticTransactionCallback : public WriteCallback {
  public:
-  explicit OptimisticTransactionCallback(OptimisticTransactionImpl* txn)
+  explicit OptimisticTransactionCallback(OptimisticTransaction* txn)
       : txn_(txn) {}
 
   Status Callback(DB* db) override {
@@ -90,7 +90,7 @@ class OptimisticTransactionCallback : public WriteCallback {
   bool AllowWriteBatching() override { return false; }
 
  private:
-  OptimisticTransactionImpl* txn_;
+  OptimisticTransaction* txn_;
 };
 
 }  // namespace rocksdb

utilities/transactions/optimistic_transaction_db_impl.cc

@@ -14,7 +14,7 @@
 #include "rocksdb/db.h"
 #include "rocksdb/options.h"
 #include "rocksdb/utilities/optimistic_transaction_db.h"
-#include "utilities/transactions/optimistic_transaction_impl.h"
+#include "utilities/transactions/optimistic_transaction.h"
 
 namespace rocksdb {
 
@@ -25,7 +25,7 @@ Transaction* OptimisticTransactionDBImpl::BeginTransaction(
     ReinitializeTransaction(old_txn, write_options, txn_options);
     return old_txn;
   } else {
-    return new OptimisticTransactionImpl(this, write_options, txn_options);
+    return new OptimisticTransaction(this, write_options, txn_options);
   }
 }
 
@@ -81,8 +81,8 @@ Status OptimisticTransactionDB::Open(
 void OptimisticTransactionDBImpl::ReinitializeTransaction(
     Transaction* txn, const WriteOptions& write_options,
     const OptimisticTransactionOptions& txn_options) {
-  assert(dynamic_cast<OptimisticTransactionImpl*>(txn) != nullptr);
-  auto txn_impl = reinterpret_cast<OptimisticTransactionImpl*>(txn);
+  assert(dynamic_cast<OptimisticTransaction*>(txn) != nullptr);
+  auto txn_impl = reinterpret_cast<OptimisticTransaction*>(txn);
 
   txn_impl->Reinitialize(this, write_options, txn_options);
 }