merge origin into feature branch

arangodb · mchacki · Sep 10, 2021 · Apr 22, 2021 · Apr 22, 2021 · Apr 22, 2021
commit cea612e7c4bda45a2b32dddc45488a859e5ff969
diff --git a/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp b/arangod/Aql/EngineInfoContainerDBServerServerBased.cpp
@@ -422,7 +422,7 @@ Result EngineInfoContainerDBServerServerBased::buildEngines(
         futures::collectAll(requests).wait();
       }
     } catch (std::exception const& ex) {
-      LOG_TOPIC("2a9fe", WARN, Logger::AQL) 
+      LOG_TOPIC("2a9fe", WARN, Logger::AQL)
         << "unable to clean up query snippets: " << ex.what();
     }
   });
@@ -433,7 +433,7 @@ Result EngineInfoContainerDBServerServerBased::buildEngines(
     // nullptr only happens on controlled shutdown
     return {TRI_ERROR_SHUTTING_DOWN};
   }
-  
+
   // remember which servers we add during our setup request
   ::arangodb::containers::HashSet<std::string> serversAdded;
 
@@ -483,13 +483,6 @@ Result EngineInfoContainerDBServerServerBased::buildEngines(
       continue;
     }
 
-    if (!trx.state()->knownServers().contains(server)) {
-      // we are about to add this server to the transaction.
-      // remember it, so we can roll the addition back for
-      // the second setup request if we need to
-      serversAdded.emplace(server);
-    }
-
     networkCalls.emplace_back(
         buildSetupRequest(trx, server, infoSlice, didCreateEngine, snippetIds,
                           serverToQueryId, serverToQueryIdLock, pool, options));
@@ -503,7 +496,7 @@ Result EngineInfoContainerDBServerServerBased::buildEngines(
             // We can directly report a non TRI_ERROR_LOCK_TIMEOUT
             // error as we need to abort after.
             // Otherwise we need to report
-            Result res{TRI_ERROR_NO_ERROR};
+            Result res;
             for (auto const& tryRes : responses) {
               auto response = tryRes.get();
               if (response.fail()) {
@@ -530,20 +523,19 @@ Result EngineInfoContainerDBServerServerBased::buildEngines(
       return fastPathResult.get();
     }
 
-    // we got a lock timeout response for the fast path locking...
     {
       // in case of fast path failure, we need to cleanup engines
       auto requests = cleanupEngines(fastPathResult.get().errorNumber(),
                                      _query.vocbase().name(), serverToQueryId);
-      // Wait for all requests to complete.
+      // Wait for all cleanup requests to complete.
       // So we know that all Transactions are aborted.
       Result res;
       for (auto& tryRes : requests) {
-        network::Response const& response = tryRes.get(); 
+        network::Response const& response = tryRes.get();
         if (response.fail()) {
           // note first error, but continue iterating over all results
           LOG_TOPIC("2d319", DEBUG, Logger::AQL)
-              << "received error from server " << response.destination 
+              << "received error from server " << response.destination
               << " during query cleanup: " << response.combinedResult().errorMessage();
           res.reset(response.combinedResult());
         }
@@ -557,6 +549,14 @@ Result EngineInfoContainerDBServerServerBased::buildEngines(
       }
     }
 
+    // fast path locking rolled back successfully!
+    snippetIds.clear();
+
+    // revert the addition of servers by us
+    for (auto const& s : serversAdded) {
+      trx.state()->removeKnownServer(s);
+    }
+
     // we must generate a new query id, because the fast path setup has failed
     clusterQueryId =
         _query.vocbase().server().getFeature<ClusterFeature>().clusterInfo().uniqid();
@@ -577,6 +577,7 @@ Result EngineInfoContainerDBServerServerBased::buildEngines(
         << "Potential deadlock detected, using slow path for locking. This "
            "is expected if exclusive locks are used.";
 
+    trx.state()->coordinatorRerollTransactionId();
 
     // Make sure we always use the same ordering on servers
     std::sort(engineInformation.begin(), engineInformation.end(),
@@ -744,7 +745,7 @@ std::vector<arangodb::network::FutureRes> EngineInfoContainerDBServerServerBased
   options.database = dbname;
   options.timeout = network::Timeout(10.0);  // Picked arbitrarily
   options.skipScheduler = true;              // hack to speed up future.get()
-
+    
   // Shutdown query snippets
   std::string url("/_api/aql/finish/");
   VPackBuffer<uint8_t> body;