Remove QueryInfo from DispatchManager

QueryInfo is a large class which is expensive to serialize, and is not necessary prior to query execution.
prestodb · Sep 2, 2020 · 5d6ec74 · 5d6ec74
1 parent 4375cd2
commit 5d6ec74
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 124 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/dispatcher/DispatchManager.java b/presto-main/src/main/java/com/facebook/presto/dispatcher/DispatchManager.java
@@ -272,11 +272,6 @@ public BasicQueryInfo getQueryInfo(QueryId queryId)
         return queryTracker.getQuery(queryId).getBasicQueryInfo();
     }
 
-    public QueryInfo getFullQueryInfo(QueryId queryId)
-    {
-        return queryTracker.getQuery(queryId).getQueryInfo();
-    }
-
     public Optional<DispatchInfo> getDispatchInfo(QueryId queryId)
     {
         return queryTracker.tryGetQuery(queryId)

diff --git a/presto-main/src/main/java/com/facebook/presto/dispatcher/FailedDispatchQuery.java b/presto-main/src/main/java/com/facebook/presto/dispatcher/FailedDispatchQuery.java
@@ -15,7 +15,6 @@
 
 import com.facebook.presto.Session;
 import com.facebook.presto.execution.ExecutionFailureInfo;
-import com.facebook.presto.execution.QueryInfo;
 import com.facebook.presto.execution.QueryState;
 import com.facebook.presto.execution.StateMachine.StateChangeListener;
 import com.facebook.presto.server.BasicQueryInfo;
@@ -31,8 +30,8 @@
 import java.util.Optional;
 import java.util.concurrent.Executor;
 
-import static com.facebook.presto.execution.QueryInfo.immediateFailureQueryInfo;
 import static com.facebook.presto.execution.QueryState.FAILED;
+import static com.facebook.presto.server.BasicQueryInfo.immediateFailureQueryInfo;
 import static com.google.common.util.concurrent.Futures.immediateFuture;
 import static io.airlift.units.DataSize.Unit.BYTE;
 import static java.util.Objects.requireNonNull;
@@ -41,7 +40,6 @@
 public class FailedDispatchQuery
         implements DispatchQuery
 {
-    private final QueryInfo queryInfo;
     private final BasicQueryInfo basicQueryInfo;
     private final Session session;
     private final Executor executor;
@@ -62,15 +60,14 @@ public FailedDispatchQuery(
         requireNonNull(failure, "failure is null");
         requireNonNull(executor, "executor is null");
 
-        this.queryInfo = immediateFailureQueryInfo(session, query, self, resourceGroup, failure);
-        this.basicQueryInfo = new BasicQueryInfo(queryInfo);
+        this.basicQueryInfo = immediateFailureQueryInfo(session, query, self, resourceGroup, failure);
         this.session = requireNonNull(session, "session is null");
         this.executor = requireNonNull(executor, "executor is null");
 
         this.dispatchInfo = DispatchInfo.failed(
                 failure,
-                queryInfo.getQueryStats().getElapsedTime(),
-                queryInfo.getQueryStats().getQueuedTime());
+                basicQueryInfo.getQueryStats().getElapsedTime(),
+                basicQueryInfo.getQueryStats().getQueuedTime());
     }
 
     @Override
@@ -79,12 +76,6 @@ public BasicQueryInfo getBasicQueryInfo()
         return basicQueryInfo;
     }
 
-    @Override
-    public QueryInfo getQueryInfo()
-    {
-        return queryInfo;
-    }
-
     @Override
     public Session getSession()
     {
@@ -124,7 +115,7 @@ public void pruneInfo() {}
     @Override
     public QueryId getQueryId()
     {
-        return queryInfo.getQueryId();
+        return basicQueryInfo.getQueryId();
     }
 
     @Override
@@ -136,7 +127,7 @@ public boolean isDone()
     @Override
     public Optional<ErrorCode> getErrorCode()
     {
-        return Optional.ofNullable(queryInfo.getErrorCode());
+        return Optional.ofNullable(basicQueryInfo.getErrorCode());
     }
 
     @Override
@@ -145,13 +136,13 @@ public void recordHeartbeat() {}
     @Override
     public DateTime getLastHeartbeat()
     {
-        return queryInfo.getQueryStats().getEndTime();
+        return basicQueryInfo.getQueryStats().getEndTime();
     }
 
     @Override
     public DateTime getCreateTime()
     {
-        return queryInfo.getQueryStats().getCreateTime();
+        return basicQueryInfo.getQueryStats().getCreateTime();
     }
 
     @Override
@@ -163,7 +154,7 @@ public Optional<DateTime> getExecutionStartTime()
     @Override
     public Optional<DateTime> getEndTime()
     {
-        return Optional.ofNullable(queryInfo.getQueryStats().getEndTime());
+        return Optional.ofNullable(basicQueryInfo.getQueryStats().getEndTime());
     }
 
     @Override

diff --git a/presto-main/src/main/java/com/facebook/presto/dispatcher/LocalDispatchQuery.java b/presto-main/src/main/java/com/facebook/presto/dispatcher/LocalDispatchQuery.java
@@ -19,7 +19,6 @@
 import com.facebook.presto.execution.ClusterSizeMonitor;
 import com.facebook.presto.execution.ExecutionFailureInfo;
 import com.facebook.presto.execution.QueryExecution;
-import com.facebook.presto.execution.QueryInfo;
 import com.facebook.presto.execution.QueryState;
 import com.facebook.presto.execution.QueryStateMachine;
 import com.facebook.presto.execution.StateMachine.StateChangeListener;
@@ -225,14 +224,6 @@ public BasicQueryInfo getBasicQueryInfo()
                 .orElse(stateMachine.getBasicQueryInfo(Optional.empty()));
     }
 
-    @Override
-    public QueryInfo getQueryInfo()
-    {
-        return tryGetQueryExecution()
-                .map(QueryExecution::getQueryInfo)
-                .orElse(stateMachine.getQueryInfo(Optional.empty()));
-    }
-
     @Override
     public Session getSession()
     {
@@ -251,10 +242,10 @@ public void fail(Throwable throwable)
     public void cancel()
     {
         if (stateMachine.transitionToCanceled()) {
-            QueryInfo queryInfo = stateMachine.getQueryInfo(Optional.empty());
+            BasicQueryInfo queryInfo = stateMachine.getBasicQueryInfo(Optional.empty());
             ExecutionFailureInfo failureInfo = queryInfo.getFailureInfo();
             failureInfo = failureInfo != null ? failureInfo : toFailure(new PrestoException(USER_CANCELED, "Query was canceled"));
-            queryMonitor.queryImmediateFailureEvent(new BasicQueryInfo(queryInfo), failureInfo);
+            queryMonitor.queryImmediateFailureEvent(queryInfo, failureInfo);
         }
     }
 

diff --git a/presto-main/src/main/java/com/facebook/presto/execution/ManagedQueryExecution.java b/presto-main/src/main/java/com/facebook/presto/execution/ManagedQueryExecution.java
@@ -45,8 +45,6 @@ public interface ManagedQueryExecution
 
     BasicQueryInfo getBasicQueryInfo();
 
-    QueryInfo getQueryInfo();
-
     boolean isDone();
 
     /**

diff --git a/presto-main/src/main/java/com/facebook/presto/server/BasicQueryInfo.java b/presto-main/src/main/java/com/facebook/presto/server/BasicQueryInfo.java
@@ -13,7 +13,9 @@
  */
 package com.facebook.presto.server;
 
+import com.facebook.presto.Session;
 import com.facebook.presto.SessionRepresentation;
+import com.facebook.presto.execution.ExecutionFailureInfo;
 import com.facebook.presto.execution.QueryInfo;
 import com.facebook.presto.execution.QueryState;
 import com.facebook.presto.spi.ErrorCode;
@@ -25,6 +27,7 @@
 import com.facebook.presto.spi.resourceGroups.ResourceGroupId;
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.collect.ImmutableList;
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
@@ -33,6 +36,9 @@
 import java.util.List;
 import java.util.Optional;
 
+import static com.facebook.presto.execution.QueryState.FAILED;
+import static com.facebook.presto.memory.LocalMemoryManager.GENERAL_POOL;
+import static com.facebook.presto.server.BasicQueryStats.immediateFailureQueryStats;
 import static com.google.common.base.MoreObjects.toStringHelper;
 import static java.util.Objects.requireNonNull;
 
@@ -88,6 +94,35 @@ public BasicQueryInfo(
         this.warnings = requireNonNull(warnings, "warnings is null");
     }
 
+    public BasicQueryInfo(
+            QueryId queryId,
+            SessionRepresentation session,
+            Optional<ResourceGroupId> resourceGroupId,
+            QueryState state,
+            MemoryPoolId memoryPool,
+            boolean scheduled,
+            URI self,
+            String query,
+            BasicQueryStats queryStats,
+            ExecutionFailureInfo failureInfo,
+            Optional<QueryType> queryType,
+            List<PrestoWarning> warnings)
+    {
+        this(
+                queryId,
+                session,
+                resourceGroupId,
+                state,
+                memoryPool,
+                scheduled,
+                self,
+                query,
+                queryStats,
+                failureInfo != null && failureInfo.getErrorCode() != null ? failureInfo.getErrorCode().getType() : null,
+                failureInfo != null ? failureInfo.getErrorCode() : null,
+                queryType, warnings);
+    }
+
     public BasicQueryInfo(QueryInfo queryInfo)
     {
         this(queryInfo.getQueryId(),
@@ -105,6 +140,23 @@ public BasicQueryInfo(QueryInfo queryInfo)
                 queryInfo.getWarnings());
     }
 
+    public static BasicQueryInfo immediateFailureQueryInfo(Session session, String query, URI self, Optional<ResourceGroupId> resourceGroupId, ExecutionFailureInfo failure)
+    {
+        return new BasicQueryInfo(
+                session.getQueryId(),
+                session.toSessionRepresentation(),
+                resourceGroupId,
+                FAILED,
+                GENERAL_POOL,
+                false,
+                self,
+                query,
+                immediateFailureQueryStats(),
+                failure,
+                Optional.empty(),
+                ImmutableList.of());
+    }
+
     @JsonProperty
     public QueryId getQueryId()
     {

diff --git a/presto-main/src/main/java/com/facebook/presto/server/BasicQueryStats.java b/presto-main/src/main/java/com/facebook/presto/server/BasicQueryStats.java
@@ -28,7 +28,9 @@
 import java.util.Set;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static io.airlift.units.DataSize.Unit.BYTE;
 import static java.util.Objects.requireNonNull;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 /**
  * Lightweight version of QueryStats. Parts of the web UI depend on the fields
@@ -163,6 +165,36 @@ public BasicQueryStats(QueryStats queryStats)
                 queryStats.getProgressPercentage());
     }
 
+    public static BasicQueryStats immediateFailureQueryStats()
+    {
+        DateTime now = DateTime.now();
+        return new BasicQueryStats(
+                now,
+                now,
+                new Duration(0, MILLISECONDS),
+                new Duration(0, MILLISECONDS),
+                new Duration(0, MILLISECONDS),
+                0,
+                0,
+                0,
+                0,
+                0,
+                new DataSize(0, BYTE),
+                0,
+                0,
+                new DataSize(0, BYTE),
+                new DataSize(0, BYTE),
+                new DataSize(0, BYTE),
+                new DataSize(0, BYTE),
+                new DataSize(0, BYTE),
+                new Duration(0, MILLISECONDS),
+                new Duration(0, MILLISECONDS),
+                false,
+                ImmutableSet.of(),
+                new DataSize(0, BYTE),
+                OptionalDouble.empty());
+    }
+
     @JsonProperty
     public DateTime getCreateTime()
     {

diff --git a/presto-main/src/main/java/com/facebook/presto/server/QueryResource.java b/presto-main/src/main/java/com/facebook/presto/server/QueryResource.java
@@ -81,11 +81,17 @@ public Response getQueryInfo(@PathParam("queryId") QueryId queryId)
         requireNonNull(queryId, "queryId is null");
 
         try {
-            QueryInfo queryInfo = dispatchManager.getFullQueryInfo(queryId);
+            QueryInfo queryInfo = queryManager.getFullQueryInfo(queryId);
             return Response.ok(queryInfo).build();
         }
         catch (NoSuchElementException e) {
-            return Response.status(Status.GONE).build();
+            try {
+                BasicQueryInfo basicQueryInfo = dispatchManager.getQueryInfo(queryId);
+                return Response.ok(basicQueryInfo).build();
+            }
+            catch (NoSuchElementException ex) {
+                return Response.status(Status.GONE).build();
+            }
         }
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -45,8 +45,6 @@ public interface ManagedQueryExecution @@
         BasicQueryInfo getBasicQueryInfo();
-        QueryInfo getQueryInfo();
         boolean isDone();
         /**
@@ Expand Down @@