fixed invalid variable and added code comments

arangodb · jsteemann · Jul 20, 2021 · Jul 16, 2021 · Jul 16, 2021 · Jul 16, 2021
commit 3e8fa22979ef5d52149047f5dd36fd6722dc6150
diff --git a/arangod/RocksDBEngine/RocksDBCollection.cpp b/arangod/RocksDBEngine/RocksDBCollection.cpp
@@ -740,7 +740,7 @@ Result RocksDBCollection::truncate(transaction::Methods& trx, OperationOptions&
     // delete documents
     RocksDBKeyBounds bounds = RocksDBKeyBounds::CollectionDocuments(objectId());
     rocksdb::Status s =
-    batch.DeleteRange(bounds.columnFamily(), bounds.start(), bounds.end());
+      batch.DeleteRange(bounds.columnFamily(), bounds.start(), bounds.end());
     if (!s.ok()) {
       return rocksutils::convertStatus(s);
     }

diff --git a/arangod/RocksDBEngine/RocksDBMetadata.cpp b/arangod/RocksDBEngine/RocksDBMetadata.cpp
@@ -100,6 +100,8 @@ RocksDBMetadata::RocksDBMetadata()
  * @param  seq   The sequence number immediately prior to call
  */
 rocksdb::SequenceNumber RocksDBMetadata::placeBlocker(TransactionId trxId, rocksdb::SequenceNumber seq) {
+  TRI_ASSERT(trxId.isSet());
+
   WRITE_LOCKER(locker, _blockerLock);
 
   seq = std::max(seq, _maxBlockersSequenceNumber);
@@ -115,14 +117,15 @@ rocksdb::SequenceNumber RocksDBMetadata::placeBlocker(TransactionId trxId, rocks
     if (!_blockersBySeq.emplace(seq, trxId).second) {
       THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "duplicate sequence number for crosslist in placeBlocker");
     }
-    LOG_TOPIC("1587a", TRACE, Logger::ENGINES)
-        << "[" << this << "] placed blocker (" << trxId.id() << ", " << seq << ")";
   } catch (...) {
     _blockers.erase(trxId);
     throw;
   }
 
   _maxBlockersSequenceNumber = seq;
+
+  LOG_TOPIC("1587a", TRACE, Logger::ENGINES)
+      << "[" << this << "] placed blocker (" << trxId.id() << ", " << seq << ")";
   return seq;
 }
 
@@ -790,7 +793,8 @@ rocksdb::SequenceNumber RocksDBBlockerGuard::placeBlocker(TransactionId trxId) {
 
   auto* rcoll = static_cast<RocksDBMetaCollection*>(_collection->getPhysical());
   // placeBlocker() may increase the blockerSeq
-  blockerSeq = rcoll->meta().placeBlocker(_trxId, blockerSeq);
+  blockerSeq = rcoll->meta().placeBlocker(trxId, blockerSeq);
+  // only set _trxId if placing the blocker succeeded
   _trxId = trxId;
   return blockerSeq;
 }

diff --git a/arangod/RocksDBEngine/RocksDBMetadata.h b/arangod/RocksDBEngine/RocksDBMetadata.h
@@ -173,6 +173,11 @@ struct RocksDBMetadata final {
   std::atomic<RevisionId> _revisionId;
 };
 
+/// helper class for acquiring and releasing a blocker.
+/// constructing an object of this class will do nothing, but once
+/// placeBlocker() is called, the object takes care of releasing the
+/// blocker upon destruction. An acquired blocker can also be released
+/// prematurely by calling releaseBlocker().
 class RocksDBBlockerGuard {
  public:
   explicit RocksDBBlockerGuard(LogicalCollection* collection);
@@ -182,7 +187,13 @@ class RocksDBBlockerGuard {
   RocksDBBlockerGuard(RocksDBBlockerGuard&&) noexcept;
   RocksDBBlockerGuard& operator=(RocksDBBlockerGuard&&) noexcept;
 
+  /// @brief can be called with or without a transaction id.
+  /// it is not allowed to call placeBlocker() if a blocker is already
+  /// acquired by the object.
   rocksdb::SequenceNumber placeBlocker(TransactionId id = TransactionId::none());
+
+  /// @brief releases an acquired blocker. will do nothing if no
+  /// blocker is currently acquired by the object.
   void releaseBlocker() noexcept;
 
  private: