arangodb · jsteemann · Aug 26, 2021 · Aug 24, 2021 · Aug 25, 2021 · Aug 26, 2021
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,10 @@
 devel
 -----
 
+* Reduce internal priority of AQL execution. This prevents possible deadlocks
+  with modification operations in a cluster and replicationFactor >= 2, and can
+  also improve responsiveness under high load of AQL queries.
+
 * Fix a potential multi-threading issue in index creation on coordinators,
   when an agency callback was triggered at the same time the method
   `ensureIndexCoordinatorInner` was left.

diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp
@@ -1265,7 +1265,7 @@ futures::Future<Result> finishDBServerParts(Query& query, ErrorCode errorCode) {
   network::RequestOptions options;
   options.database = query.vocbase().name();
   options.timeout = network::Timeout(60.0);  // Picked arbitrarily
-  options.continuationLane = RequestLane::CLUSTER_AQL_CONTINUATION;
+  options.continuationLane = RequestLane::CLUSTER_AQL_INTERNAL_COORDINATOR;
   //  options.skipScheduler = true;
 
   VPackBuffer<uint8_t> body;

diff --git a/arangod/Aql/RestAqlHandler.cpp b/arangod/Aql/RestAqlHandler.cpp
@@ -857,3 +857,11 @@ RestStatus RestAqlHandler::handleFinishQuery(std::string const& idString) {
     generateResult(rest::ResponseCode::OK, std::move(buffer));
   }));
 }
+
+RequestLane RestAqlHandler::lane() const {
+  if (ServerState::instance()->isCoordinator()) {
+    return RequestLane::CLUSTER_AQL_INTERNAL_COORDINATOR;
+  } else {
+    return RequestLane::CLUSTER_AQL;
+  }
+}
diff --git a/arangod/Aql/RestAqlHandler.h b/arangod/Aql/RestAqlHandler.h
@@ -42,7 +42,7 @@ class RestAqlHandler : public RestVocbaseBaseHandler {
 
  public:
   char const* name() const override final { return "RestAqlHandler"; }
-  RequestLane lane() const override final { return RequestLane::CLUSTER_AQL; }
+  RequestLane lane() const override final;
   RestStatus execute() override;
   RestStatus continueExecute() override;
   void shutdownExecute(bool isFinalized) noexcept override;

diff --git a/arangod/Aql/SharedQueryState.cpp b/arangod/Aql/SharedQueryState.cpp
@@ -26,6 +26,7 @@
 #include "ApplicationFeatures/ApplicationServer.h"
 #include "Basics/Exceptions.h"
 #include "Basics/ScopeGuard.h"
+#include "Cluster/ServerState.h"
 #include "RestServer/QueryRegistryFeature.h"
 #include "Scheduler/Scheduler.h"
 #include "Scheduler/SchedulerFeature.h"
@@ -129,34 +130,37 @@ void SharedQueryState::queueHandler() {
     return;
   }
 
-  bool queued =
-      scheduler->queue(RequestLane::CLUSTER_AQL_CONTINUATION,
-                       [self = shared_from_this(), cb = _wakeupCb, v = _cbVersion]() {
-                         std::unique_lock<std::mutex> lck(self->_mutex, std::defer_lock);
-
-                         do {
-                           bool cntn = false;
-                           try {
-                             cntn = cb();
-                           } catch (...) {
-                           }
-
-                           lck.lock();
-                           if (v == self->
10000
_cbVersion) {
-                             unsigned c = self->_numWakeups--;
-                             TRI_ASSERT(c > 0);
-                             if (c == 1 || !cntn || !self->_valid) {
-                               break;
-                             }
-                           } else {
-                             return;
-                           }
-                           lck.unlock();
-                         } while (true);
-
-                         TRI_ASSERT(lck);
-                         self->queueHandler();
-                       });
+  auto const lane = ServerState::instance()->isCoordinator()
+                        ? RequestLane::CLUSTER_AQL_INTERNAL_COORDINATOR
+                        : RequestLane::CLUSTER_AQL;
+
+  bool queued = scheduler->queue(lane, [self = shared_from_this(),
+                                        cb = _wakeupCb, v = _cbVersion]() {
+    std::unique_lock<std::mutex> lck(self->_mutex, std::defer_lock);
+
+    do {
+      bool cntn = false;
+      try {
+        cntn = cb();
+      } catch (...) {
+      }
+
+      lck.lock();
+      if (v == self->_cbVersion) {
+        unsigned c = self->_numWakeups--;
+        TRI_ASSERT(c > 0);
+        if (c == 1 || !cntn || !self->_valid) {
+          break;
+        }
+      } else {
+        return;
+      }
+      lck.unlock();
+    } while (true);
+
+    TRI_ASSERT(lck);
+    self->queueHandler();
+  });
 
   if (!queued) { // just invalidate
      _wakeupCb = nullptr;
@@ -168,7 +172,7 @@ void SharedQueryState::queueHandler() {
 bool SharedQueryState::queueAsyncTask(fu2::unique_function<void()> cb) {
   Scheduler* scheduler = SchedulerFeature::SCHEDULER;
   if (scheduler) {
-    return scheduler->queue(RequestLane::CLIENT_AQL, std::move(cb));
+    return scheduler->queue(RequestLane::CLUSTER_AQL, std::move(cb));
   }
   return false;
 }
diff --git a/arangod/GeneralServer/RequestLane.h b/arangod/GeneralServer/RequestLane.h
@@ -61,15 +61,16 @@ enum class RequestLane {
   // V8 or having high priority.
   CLUSTER_INTERNAL,
 
-  // For requests from the DBserver to the Coordinator or
-  // from the Coordinator to the DBserver. Using AQL
-  // these have Medium priority.
+  // Internal AQL requests, or continuations. Low priority.
   CLUSTER_AQL,
 
-  // For future continuations initiated as part of an AQL request from the
-  // DBserver to the Coordinator or from the Coordinator to the DBserver.
-  // These have High priority.
-  CLUSTER_AQL_CONTINUATION,
+  // For requests from the DBserver to the Coordinator, and continuations on the
+  // Coordinator.
+  // These have medium priority. Because client requests made against the
+  // RestCursorHandler (with lane CLIENT_AQL) might block and need these to
+  // finish. Ongoing low priority requests can also prevent low priority lanes
+  // from being worked on, having the same effect.
+  CLUSTER_AQL_INTERNAL_COORDINATOR,
 
   // For requests from the from the Coordinator to the
   // DBserver using V8.
@@ -141,9 +142,9 @@ inline RequestPriority PriorityRequestLane(RequestLane lane) {
     case RequestLane::CLUSTER_INTERNAL:
       return RequestPriority::HIGH;
     case RequestLane::CLUSTER_AQL:
+      return RequestPriority::LOW;
+    case RequestLane::CLUSTER_AQL_INTERNAL_COORDINATOR:
       return RequestPriority::MED;
-    case RequestLane::CLUSTER_AQL_CONTINUATION:
-      return RequestPriority::HIGH;
     case RequestLane::CLUSTER_V8:
       return RequestPriority::LOW;
     case RequestLane::CLUSTER_ADMIN: