protect all properties of TransactionState

arangod/MMFiles/MMFilesTransactionCollection.cpp

@@ -117,7 +117,7 @@ void MMFilesTransactionCollection::freeOperations(transaction::Methods* activeTr
     return;
   }
   
-  bool const isSingleOperationTransaction = _transaction->_hints.has(transaction::Hints::Hint::SINGLE_OPERATION);
+  bool const isSingleOperationTransaction = _transaction->hasHint(transaction::Hints::Hint::SINGLE_OPERATION);
 
   if (mustRollback) {
     // revert all operations
@@ -151,8 +151,8 @@ void MMFilesTransactionCollection::freeOperations(transaction::Methods* activeTr
 
 bool MMFilesTransactionCollection::canAccess(AccessMode::Type accessType) const {
   if (_collection == nullptr) {
-    if (!_transaction->_hints.has(transaction::Hints::Hint::LOCK_NEVER) ||
-        !_transaction->_hints.has(transaction::Hints::Hint::NO_USAGE_LOCK)) {
+    if (!_transaction->hasHint(transaction::Hints::Hint::LOCK_NEVER) ||
+        !_transaction->hasHint(transaction::Hints::Hint::NO_USAGE_LOCK)) {
       // not opened. probably a mistake made by the caller
       return false;
     }
@@ -200,8 +200,8 @@ int MMFilesTransactionCollection::use(int nestingLevel) {
 
   if (_collection == nullptr) {
     // open the collection
-    if (!_transaction->_hints.has(transaction::Hints::Hint::LOCK_NEVER) &&
-        !_transaction->_hints.has(transaction::Hints::Hint::NO_USAGE_LOCK)) {
+    if (!_transaction->hasHint(transaction::Hints::Hint::LOCK_NEVER) &&
+        !_transaction->hasHint(transaction::Hints::Hint::NO_USAGE_LOCK)) {
       // use and usage-lock
       TRI_vocbase_col_status_e status;
       LOG_TRX(_transaction, nestingLevel) << "using collection " << _cid;
@@ -241,7 +241,7 @@ int MMFilesTransactionCollection::use(int nestingLevel) {
   if (nestingLevel == 0 &&
       AccessMode::isWriteOrExclusive(_accessType)) {
     // read-lock the compaction lock
-    if (!_transaction->_hints.has(transaction::Hints::Hint::NO_COMPACTION_LOCK)) {
+    if (!_transaction->hasHint(transaction::Hints::Hint::NO_COMPACTION_LOCK)) {
       if (!_compactionLocked) {
         logicalToMMFiles(_collection)->preventCompaction();
         _compactionLocked = true;
@@ -254,10 +254,10 @@ int MMFilesTransactionCollection::use(int nestingLevel) {
     _originalRevision = _collection->revision();
   }
 
-  bool shouldLock = _transaction->_hints.has(transaction::Hints::Hint::LOCK_ENTIRELY);
+  bool shouldLock = _transaction->hasHint(transaction::Hints::Hint::LOCK_ENTIRELY);
 
   if (!shouldLock) {
-    shouldLock = (AccessMode::isWriteOrExclusive(_accessType) && !_transaction->_hints.has(transaction::Hints::Hint::SINGLE_OPERATION));
+    shouldLock = (AccessMode::isWriteOrExclusive(_accessType) && !_transaction->hasHint(transaction::Hints::Hint::SINGLE_OPERATION));
   }
 
   if (shouldLock && !isLocked()) {
@@ -281,7 +281,7 @@ void MMFilesTransactionCollection::unuse(int nestingLevel) {
 
   // the top level transaction releases all collections
   if (nestingLevel == 0 && _collection != nullptr) {
-    if (!_transaction->_hints.has(transaction::Hints::Hint::NO_COMPACTION_LOCK)) {
+    if (!_transaction->hasHint(transaction::Hints::Hint::NO_COMPACTION_LOCK)) {
       if (AccessMode::isWriteOrExclusive(_accessType) && _compactionLocked) {
         // read-unlock the compaction lock
         logicalToMMFiles(_collection)->allowCompaction();
@@ -306,7 +306,7 @@ void MMFilesTransactionCollection::release() {
 
 /// @brief lock a collection
 int MMFilesTransactionCollection::doLock(AccessMode::Type type, int nestingLevel) {
-  if (_transaction->_hints.has(transaction::Hints::Hint::LOCK_NEVER)) {
+  if (_transaction->hasHint(transaction::Hints::Hint::LOCK_NEVER)) {
     // never lock
     return TRI_ERROR_NO_ERROR;
   }
@@ -333,12 +333,12 @@ int MMFilesTransactionCollection::doLock(AccessMode::Type type, int nestingLevel
   TRI_ASSERT(physical != nullptr);
 
   double timeout = _transaction->timeout();
-  if (_transaction->_hints.has(transaction::Hints::Hint::TRY_LOCK)) {
+  if (_transaction->hasHint(transaction::Hints::Hint::TRY_LOCK)) {
     // give up early if we cannot acquire the lock instantly
     timeout = 0.00000001;
   }
   
-  bool const useDeadlockDetector = !_transaction->_hints.has(transaction::Hints::Hint::SINGLE_OPERATION);
+  bool const useDeadlockDetector = !_transaction->hasHint(transaction::Hints::Hint::SINGLE_OPERATION);
 
   int res;
   if (!isWrite(type)) {
@@ -358,7 +358,7 @@ int MMFilesTransactionCollection::doLock(AccessMode::Type type, int nestingLevel
 
 /// @brief unlock a collection
 int MMFilesTransactionCollection::doUnlock(AccessMode::Type type, int nestingLevel) {
-  if (_transaction->_hints.has(transaction::Hints::Hint::LOCK_NEVER)) {
+  if (_transaction->hasHint(transaction::Hints::Hint::LOCK_NEVER)) {
     // never unlock
     return TRI_ERROR_NO_ERROR;
   }
@@ -394,7 +394,7 @@ int MMFilesTransactionCollection::doUnlock(AccessMode::Type type, int nestingLev
     return TRI_ERROR_INTERNAL;
   }
 
-  bool const useDeadlockDetector = !_transaction->_hints.has(transaction::Hints::Hint::SINGLE_OPERATION);
+  bool const useDeadlockDetector = !_transaction->hasHint(transaction::Hints::Hint::SINGLE_OPERATION);
 
   LogicalCollection* collection = _collection;
   TRI_ASSERT(collection != nullptr);

arangod/MMFiles/MMFilesTransactionState.cpp

@@ -53,6 +53,7 @@ static inline MMFilesLogfileManager* GetMMFilesLogfileManager() {
 MMFilesTransactionState::MMFilesTransactionState(TRI_vocbase_t* vocbase)
     : TransactionState(vocbase),
       _rocksTransaction(nullptr),
+      _beginWritten(false),
       _hasOperations(false) {}
 
 /// @brief free a transaction container

arangod/MMFiles/MMFilesTransactionState.h

@@ -106,6 +106,7 @@ class MMFilesTransactionState final : public TransactionState {
   
  private:
   rocksdb::Transaction* _rocksTransaction;
+  bool _beginWritten;
   bool _hasOperations;
 };
 

arangod/StorageEngine/TransactionCollection.h

@@ -33,13 +33,12 @@ class LogicalCollection;
 namespace transaction {
 class Methods;
 }
-;
+
 class TransactionState;
 
 /// @brief collection used in a transaction
 class TransactionCollection {
  public:
-
   TransactionCollection(TransactionCollection const&) = delete;
   TransactionCollection& operator=(TransactionCollection const&) = delete;
 

arangod/StorageEngine/TransactionState.cpp

@@ -43,11 +43,10 @@ TransactionState::TransactionState(TRI_vocbase_t* vocbase)
       _collections{_arena}, // assign arena to vector 
       _serverRole(ServerState::instance()->getRole()),
       _hints(),
+      _timeout(transaction::Methods::DefaultLockTimeout),
       _nestingLevel(0), 
       _allowImplicitCollections(true),
-      _waitForSync(false),
-      _beginWritten(false), 
-      _timeout(transaction::Methods::DefaultLockTimeout) {}
+      _waitForSync(false) {}
 
 /// @brief free a transaction container
 TransactionState::~TransactionState() {

arangod/StorageEngine/TransactionState.h

@@ -27,7 +27,6 @@
 #include "Basics/Common.h"
 #include "Basics/SmallVector.h"
 #include "Cluster/ServerState.h"
-#include "Transaction/Methods.h"
 #include "Transaction/Hints.h"
 #include "Transaction/Status.h"
 #include "VocBase/AccessMode.h"
@@ -50,7 +49,6 @@ namespace arangodb {
 namespace transaction {
 class Methods;
 }
-;
 class TransactionCollection;
 
 /// @brief transaction type
@@ -71,6 +69,12 @@ class TransactionState {
   TRI_voc_tid_t id() const { return _id; }
   transaction::Status status() const { return _status; }
 
+  int increaseNesting() { return ++_nestingLevel; }
+  int decreaseNesting() { 
+    TRI_ASSERT(_nestingLevel > 0);
+    return --_nestingLevel; 
+  }
+
   double timeout() const { return _timeout; }
   void timeout(double value) { 
     if (value > 0.0) {
@@ -112,9 +116,10 @@ class TransactionState {
   void updateStatus(transaction::Status status);
   
   /// @brief whether or not a specific hint is set for the transaction
-  bool hasHint(transaction::Hints::Hint hint) const {
-    return _hints.has(hint);
-  }
+  bool hasHint(transaction::Hints::Hint hint) const { return _hints.has(hint); }
+  
+  /// @brief set a hint for the transaction
+  void setHint(transaction::Hints::Hint hint) { _hints.set(hint); }
 
   /// @brief begin a transaction
   virtual int beginTransaction(transaction::Hints hints, int nestingLevel) = 0;
@@ -125,7 +130,6 @@ class TransactionState {
   /// @brief abort a transaction
   virtual int abortTransaction(transaction::Methods* trx, int nestingLevel) = 0;
 
-  /// TODO: implement this in base class
   virtual bool hasFailedOperations() const = 0;
 
  protected:
@@ -158,16 +162,11 @@ class TransactionState {
   
   ServerState::RoleEnum const _serverRole;  // role of the server
 
- public:
   transaction::Hints _hints;          // hints;
- public:
+  double _timeout;                    // timeout for lock acquisition
   int _nestingLevel;
-
- protected:
   bool _allowImplicitCollections;
   bool _waitForSync;   // whether or not the transaction had a synchronous op
-  bool _beginWritten;  // whether or not the begin marker was already written
-  double _timeout;     // timeout for lock acquisition
 };
 
 }

arangod/Transaction/Methods.cpp

@@ -167,7 +167,7 @@ void transaction::Methods::addHint(transaction::Hints::Hint hint, bool passthrou
   _hints.set(hint);
 
   if (passthrough) {
-    _state->_hints.set(hint);
+    _state->setHint(hint);
   }
 }
   
@@ -580,7 +580,7 @@ transaction::Methods::Methods(std::shared_ptr<TransactionContext> transactionCon
 /// @brief destroy the transaction
 transaction::Methods::~Methods() {
   if (isEmbeddedTransaction()) {
-    _state->_nestingLevel--;
+    _state->decreaseNesting();
   } else {
     if (getStatus() == transaction::Status::RUNNING) {
       // auto abort a running transaction
@@ -2904,7 +2904,7 @@ void transaction::Methods::setupEmbedded(TRI_vocbase_t*) {
   _state = _transactionContextPtr->getParentTransaction();
   
   TRI_ASSERT(_state != nullptr);
-  _nestingLevel = ++_state->_nestingLevel;
+  _nestingLevel = _state->increaseNesting();
 }
 
 /// @brief set up a top-level transaction