From b9d8db7b1cbce2373e437cb45c5b21c4d2b99c63 Mon Sep 17 00:00:00 2001 From: Dain Sundstrom Date: Tue, 6 Nov 2018 19:05:48 -0800 Subject: [PATCH] Produce all query timings for query regardless of state Instead of delaying production of state timings until each state is complete, produce final duration for completed states, partial duration for current state, and zero duration for future states. This simplifies the code, and always gives up to date data for the current state. This does not have a large impact as these stats are use for informational purposes and for final events (which would have final data). --- .../facebook/presto/event/QueryMonitor.java | 6 +-- .../presto/execution/QueryStateMachine.java | 10 ++-- .../presto/execution/QueryStateTimer.java | 20 ++++---- .../facebook/presto/execution/QueryStats.java | 22 ++++---- .../presto/server/BasicQueryStats.java | 6 +-- .../execution/TestQueryStateMachine.java | 50 +++---------------- .../single_query_info_response.json | 3 ++ 7 files changed, 39 insertions(+), 78 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/event/QueryMonitor.java b/presto-main/src/main/java/com/facebook/presto/event/QueryMonitor.java index ad520db95afe..0aed58742b6a 100644 --- a/presto-main/src/main/java/com/facebook/presto/event/QueryMonitor.java +++ b/presto-main/src/main/java/com/facebook/presto/event/QueryMonitor.java @@ -210,8 +210,8 @@ private QueryStatistics createQueryStatistics(QueryInfo queryInfo) ofMillis(queryStats.getTotalCpuTime().toMillis()), ofMillis(queryStats.getTotalScheduledTime().toMillis()), ofMillis(queryStats.getQueuedTime().toMillis()), - Optional.ofNullable(queryStats.getAnalysisTime()).map(duration -> ofMillis(duration.toMillis())), - Optional.ofNullable(queryStats.getDistributedPlanningTime()).map(duration -> ofMillis(duration.toMillis())), + Optional.of(ofMillis(queryStats.getAnalysisTime().toMillis())), + Optional.of(ofMillis(queryStats.getDistributedPlanningTime().toMillis())), queryStats.getPeakUserMemoryReservation().toBytes(), queryStats.getPeakTotalMemoryReservation().toBytes(), queryStats.getPeakTaskTotalMemory().toBytes(), @@ -363,7 +363,7 @@ private static void logQueryTimeline(QueryInfo queryInfo) } // planning duration -- start to end of planning - long planning = max(queryStats.getTotalPlanningTime() == null ? 0 : queryStats.getTotalPlanningTime().toMillis(), 0); + long planning = queryStats.getTotalPlanningTime().toMillis(); List stages = StageInfo.getAllStages(queryInfo.getOutputStage()); // long lastSchedulingCompletion = 0; diff --git a/presto-main/src/main/java/com/facebook/presto/execution/QueryStateMachine.java b/presto-main/src/main/java/com/facebook/presto/execution/QueryStateMachine.java index f77177930219..5f716b2d622a 100644 --- a/presto-main/src/main/java/com/facebook/presto/execution/QueryStateMachine.java +++ b/presto-main/src/main/java/com/facebook/presto/execution/QueryStateMachine.java @@ -471,12 +471,12 @@ private QueryStats getQueryStats(Optional rootStage) queryStateTimer.getElapsedTime(), queryStateTimer.getQueuedTime(), - queryStateTimer.getResourceWaitingTime().orElse(null), + queryStateTimer.getResourceWaitingTime(), queryStateTimer.getExecutionTime(), - queryStateTimer.getAnalysisTime().orElse(null), - queryStateTimer.getDistributedPlanningTime().orElse(null), - queryStateTimer.getPlanningTime().orElse(null), - queryStateTimer.getFinishingTime().orElse(null), + queryStateTimer.getAnalysisTime(), + queryStateTimer.getDistributedPlanningTime(), + queryStateTimer.getPlanningTime(), + queryStateTimer.getFinishingTime(), totalTasks, runningTasks, diff --git a/presto-main/src/main/java/com/facebook/presto/execution/QueryStateTimer.java b/presto-main/src/main/java/com/facebook/presto/execution/QueryStateTimer.java index 6595d11f7f6f..b974e1a2bfd4 100644 --- a/presto-main/src/main/java/com/facebook/presto/execution/QueryStateTimer.java +++ b/presto-main/src/main/java/com/facebook/presto/execution/QueryStateTimer.java @@ -192,19 +192,19 @@ public Duration getQueuedTime() return getElapsedTime(); } - public Optional getResourceWaitingTime() + public Duration getResourceWaitingTime() { - return Optional.ofNullable(resourceWaitingTime.get()); + return getDuration(resourceWaitingTime, beginResourceWaitingNanos); } - public Optional getPlanningTime() + public Duration getPlanningTime() { - return Optional.ofNullable(planningTime.get()); + return getDuration(planningTime, beginPlanningNanos); } - public Optional getFinishingTime() + public Duration getFinishingTime() { - return Optional.ofNullable(finishingTime.get()); + return getDuration(finishingTime, beginFinishingNanos); } public Duration getExecutionTime() @@ -217,14 +217,14 @@ public Optional getEndTime() return toDateTime(endNanos); } - public Optional getAnalysisTime() + public Duration getAnalysisTime() { - return Optional.ofNullable(analysisTime.get()); + return getDuration(analysisTime, beginAnalysisNanos); } - public Optional getDistributedPlanningTime() + public Duration getDistributedPlanningTime() { - return Optional.ofNullable(distributedPlanningTime.get()); + return getDuration(distributedPlanningTime, beginDistributedPlanningNanos); } public DateTime getLastHeartbeat() diff --git a/presto-main/src/main/java/com/facebook/presto/execution/QueryStats.java b/presto-main/src/main/java/com/facebook/presto/execution/QueryStats.java index ad570f0e01a0..b47511267a48 100644 --- a/presto-main/src/main/java/com/facebook/presto/execution/QueryStats.java +++ b/presto-main/src/main/java/com/facebook/presto/execution/QueryStats.java @@ -154,14 +154,14 @@ public QueryStats( this.lastHeartbeat = requireNonNull(lastHeartbeat, "lastHeartbeat is null"); this.endTime = endTime; - this.elapsedTime = elapsedTime; - this.queuedTime = queuedTime; - this.resourceWaitingTime = resourceWaitingTime; + this.elapsedTime = requireNonNull(elapsedTime, "elapsedTime is null"); + this.queuedTime = requireNonNull(queuedTime, "queuedTime is null"); + this.resourceWaitingTime = requireNonNull(resourceWaitingTime, "resourceWaitingTime is null"); this.executionTime = requireNonNull(executionTime, "executionTime is null"); - this.analysisTime = analysisTime; - this.distributedPlanningTime = distributedPlanningTime; - this.totalPlanningTime = totalPlanningTime; - this.finishingTime = finishingTime; + this.analysisTime = requireNonNull(analysisTime, "analysisTime is null"); + this.distributedPlanningTime = requireNonNull(distributedPlanningTime, "distributedPlanningTime is null"); + this.totalPlanningTime = requireNonNull(totalPlanningTime, "totalPlanningTime is null"); + this.finishingTime = requireNonNull(finishingTime, "finishingTime is null"); checkArgument(totalTasks >= 0, "totalTasks is negative"); this.totalTasks = totalTasks; @@ -225,8 +225,8 @@ public static QueryStats immediateFailureQueryStats() new Duration(0, MILLISECONDS), new Duration(0, MILLISECONDS), new Duration(0, MILLISECONDS), - null, - null, + new Duration(0, MILLISECONDS), + new Duration(0, MILLISECONDS), new Duration(0, MILLISECONDS), new Duration(0, MILLISECONDS), 0, @@ -300,10 +300,6 @@ public Duration getResourceWaitingTime() @JsonProperty public Duration getQueuedTime() { - if (queuedTime == null) { - // counter-intuitively, this means that the query is still queued - return elapsedTime; - } return queuedTime; } 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 index 247d11e5b116..a395f7f8bf09 100644 --- a/presto-main/src/main/java/com/facebook/presto/server/BasicQueryStats.java +++ b/presto-main/src/main/java/com/facebook/presto/server/BasicQueryStats.java @@ -90,9 +90,9 @@ public BasicQueryStats( this.createTime = createTime; this.endTime = endTime; - this.queuedTime = queuedTime; - this.elapsedTime = elapsedTime; - this.executionTime = executionTime; + this.queuedTime = requireNonNull(queuedTime, "queuedTime is null"); + this.elapsedTime = requireNonNull(elapsedTime, "elapsedTime is null"); + this.executionTime = requireNonNull(executionTime, "executionTime is null"); checkArgument(totalDrivers >= 0, "totalDrivers is negative"); this.totalDrivers = totalDrivers; diff --git a/presto-main/src/test/java/com/facebook/presto/execution/TestQueryStateMachine.java b/presto-main/src/test/java/com/facebook/presto/execution/TestQueryStateMachine.java index fb8d05566ce3..c19dda8a379c 100644 --- a/presto-main/src/test/java/com/facebook/presto/execution/TestQueryStateMachine.java +++ b/presto-main/src/test/java/com/facebook/presto/execution/TestQueryStateMachine.java @@ -166,16 +166,10 @@ private void assertAllTimeSpentInQueueing(QueryState expectedState, Consumer