Revert "Try to avoid synchronous waiting in AQL, and fix transaction …

…lifetime issues" This reverts commit c787f51.
arangodb · mpoeter · Jun 20, 2024 · Jun 6, 2024 · Jun 11, 2024 · Jun 11, 2024
commit 2bd1475e2a838d4082fd3d2787cc2b47ca57d65d
diff --git a/arangod/Aql/SimpleModifier.cpp b/arangod/Aql/SimpleModifier.cpp
@@ -186,6 +186,13 @@ ExecutionState SimpleModifier<ModifierCompletion, Enable>::transact(
 
   auto result = _completion.transact(trx, _accumulator.closeAndGetContents());
 
+  // we are currently waiting here for the `result` future to get
+  // ready before we continue. this makes the AQL modification
+  // operations blocking as in previous versions of ArangoDB.
+  // TODO: fix this and make it truly non-blocking (requires to
+  // fix some lifecycle issues for AQL queries first).
+  result.wait();
+
   if (result.isReady()) {
     _results = std::move(result.waitAndGet());
     return ExecutionState::DONE;

diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp
@@ -3604,13 +3604,11 @@ Future<Result> Methods::replicateOperations(
   // we continue with the operation, since most likely, the follower was
   // simply dropped in the meantime.
   // In any case, we drop the follower here (just in case).
-  auto cb = [followerList, startTimeReplication, opName, collection, count,
-             vocbase = vocbase().getSharedPtr(), state = _state](
-                std::vector<futures::Try<network::Response>>&& responses)
+  auto cb = [=, this](std::vector<futures::Try<network::Response>>&& responses)
       -> futures::Future<Result> {
     auto duration = std::chrono::steady_clock::now() - startTimeReplication;
     auto& replMetrics =
-        vocbase->server().getFeature<ReplicationMetricsFeature>();
+        vocbase().server().getFeature<ReplicationMetricsFeature>();
     replMetrics.synchronousOpsTotal() += 1;
     replMetrics.synchronousTimeTotal() +=
         std::chrono::nanoseconds(duration).count();
@@ -3648,7 +3646,8 @@ Future<Result> Methods::replicateOperations(
           // follower, but simply return the error and abort our local
           // transaction.
           if (r.is(TRI_ERROR_TRANSACTION_ABORTED) &&
-              state->hasHint(transaction::Hints::Hint::FROM_TOPLEVEL_AQL)) {
+              this->state()->hasHint(
+                  transaction::Hints::Hint::FROM_TOPLEVEL_AQL)) {
             return r;
           }
 
@@ -3660,7 +3659,8 @@ Future<Result> Methods::replicateOperations(
               absl::StrCat("got error from follower: ", r.errorMessage());
 
           if (followerRefused) {
-            ++vocbase->server()
+            ++vocbase()
+                  .server()
                   .getFeature<arangodb::ClusterFeature>()
                   .followersRefusedCounter();
 
@@ -3680,7 +3680,7 @@ Future<Result> Methods::replicateOperations(
       }
 
       if (!replicationFailureReason.empty()) {
-        if (!vocbase->server().isStopping()) {
+        if (!vocbase().server().isStopping()) {
           LOG_TOPIC("12d8c", WARN, Logger::REPLICATION)
               << "synchronous replication of " << opName << " operation "
               << "(" << count << " doc(s)): "
@@ -3742,7 +3742,7 @@ Future<Result> Methods::replicateOperations(
       return Result{TRI_ERROR_CLUSTER_SHARD_LEADER_RESIGNED};
     } else {
       // execute a deferred intermediate commit, if required.
-      return state->performIntermediateCommitIfRequired(collection->id());
+      return performIntermediateCommitIfRequired(collection->id());
     }
   };
   return futures::collectAll(std::move(futures)).thenValue(std::move(cb));

diff --git a/arangod/VocBase/vocbase.cpp b/arangod/VocBase/vocbase.cpp
@@ -129,10 +129,6 @@ void TRI_vocbase_t::release() noexcept {
   TRI_ASSERT(v >= 2);
 }
 
-arangodb::VocbasePtr TRI_vocbase_t::getSharedPtr() noexcept {
-  return VocbasePtr{use() ? this : nullptr};
-}
-
 bool TRI_vocbase_t::isDangling() const noexcept {
   auto const v = _refCount.load(std::memory_order_acquire);
   TRI_ASSERT((v & 1) == 0 || !isSystem());

diff --git a/arangod/VocBase/vocbase.h b/arangod/VocBase/vocbase.h
@@ -41,7 +41,6 @@
 #include "Containers/FlatHashMap.h"
 #include "Replication2/Version.h"
 #include "RestServer/arangod.h"
-#include "Utils/DatabaseGuard.h"
 #include "Utils/VersionTracker.h"
 #include "VocBase/Identifiers/DataSourceId.h"
 #include "VocBase/Identifiers/TransactionId.h"
@@ -294,8 +293,6 @@ struct TRI_vocbase_t {
   /// @brief decrease the reference counter for a database
   void release() noexcept;
 
-  arangodb::VocbasePtr getSharedPtr() noexcept;
-
   /// @brief returns whether the database is dangling
   bool isDangling() const noexcept;