From 5bf30735be4be8d747b7aab6f52078e99c204ecd Mon Sep 17 00:00:00 2001
From: Ruslan Mardugalliamov <ruslanmar@meta.com>
Date: Tue, 3 Jan 2023 08:22:27 -0700
Subject: [PATCH] Fix blocking resource groups when query queue full

When Presto Coordinator cannot run query nor queue query, it fails the error.
And Coordinator does it under root resource group lock. Failing query triggers query state change that in particular triggers event listener handler that could be expensive. Doing it under resource group lock essentially blocks the resource group and severely degrades performance.
---
 .../resourceGroups/InternalResourceGroup.java | 29 +++++++++++--------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/presto-main/src/main/java/com/facebook/presto/execution/resourceGroups/InternalResourceGroup.java b/presto-main/src/main/java/com/facebook/presto/execution/resourceGroups/InternalResourceGroup.java
index 58be7ef9e0d7a..6397b482bd89e 100644
--- a/presto-main/src/main/java/com/facebook/presto/execution/resourceGroups/InternalResourceGroup.java
+++ b/presto-main/src/main/java/com/facebook/presto/execution/resourceGroups/InternalResourceGroup.java
@@ -680,6 +680,7 @@ protected void setDirty()
 
     public void run(ManagedQueryExecution query)
     {
+        boolean isQueryQueueFull = false;
         synchronized (root) {
             if (!subGroups.isEmpty()) {
                 throw new PrestoException(INVALID_RESOURCE_GROUP, format("Cannot add queries to %s. It is not a leaf group.", id));
@@ -697,21 +698,25 @@ public void run(ManagedQueryExecution query)
                 group = group.parent.get();
             }
             if (!canQueue && !canRun) {
-                query.fail(new QueryQueueFullException(id));
-                return;
-            }
-            query.setResourceGroupQueryLimits(perQueryLimits);
-            if (canRun && queuedQueries.isEmpty()) {
-                startInBackground(query);
+                isQueryQueueFull = true;
             }
             else {
-                enqueueQuery(query);
-            }
-            query.addStateChangeListener(state -> {
-                if (state.isDone()) {
-                    queryFinished(query);
+                query.setResourceGroupQueryLimits(perQueryLimits);
+                if (canRun && queuedQueries.isEmpty()) {
+                    startInBackground(query);
                 }
-            });
+                else {
+                    enqueueQuery(query);
+                }
+                query.addStateChangeListener(state -> {
+                    if (state.isDone()) {
+                        queryFinished(query);
+                    }
+                });
+            }
+        }
+        if (isQueryQueueFull) {
+            query.fail(new QueryQueueFullException(id));
         }
     }