Skip to content

Commit 7f65e8a

Browse files
pgupta2facebook-github-bot
authored andcommitted
Support max queued_time limit (#25589)
Summary: Support Max queued time limit. Queries queued for more than this threshold should fail with EXCEEDED_TIME_LIMIT error code and proper error msg Differential Revision: D78709808
1 parent 45c3305 commit 7f65e8a

15 files changed

Lines changed: 469 additions & 4 deletions

File tree

presto-docs/src/main/sphinx/admin/properties-session.rst

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,4 +484,14 @@ The corresponding configuration property is :ref:`admin/properties:\`\`query.cli
484484
* **Default value:** ``1``
485485

486486
This property defines the priority of queries for execution and plays an important role in query admission.
487-
Queries with higher priority are scheduled first than the ones with lower priority. Higher number indicates higher priority.
487+
Queries with higher priority are scheduled first than the ones with lower priority. Higher number indicates higher priority.
488+
489+
``query_max_queued_time``
490+
^^^^^^^^^^^^^^^^^^^^^^^^^
491+
492+
* **Type:** ``Duration``
493+
* **Default value:** ``100d``
494+
495+
Use to configure how long a query can be queued before it is terminated.
496+
497+
The corresponding configuration property is :ref:`admin/properties:\`\`query.max-queued-time\`\``.

presto-docs/src/main/sphinx/admin/properties.rst

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1136,4 +1136,14 @@ Query Manager Properties
11361136
This property can be used to configure how long a query runs without contact
11371137
from the client application, such as the CLI, before it's abandoned.
11381138

1139-
The corresponding session property is :ref:`admin/properties-session:\`\`query_client_timeout\`\``.
1139+
The corresponding session property is :ref:`admin/properties-session:\`\`query_client_timeout\`\``.
1140+
1141+
``query.max-queued-time``
1142+
^^^^^^^^^^^^^^^^^^^^^^^^^
1143+
1144+
* **Type:** ``Duration``
1145+
* **Default value:** ``100d``
1146+
1147+
Use to configure how long a query can be queued before it is terminated.
1148+
1149+
The corresponding session property is :ref:`admin/properties-session:\`\`query_max_queued_time\`\``.

presto-main-base/src/main/java/com/facebook/presto/SystemSessionProperties.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ public final class SystemSessionProperties
115115
public static final String QUERY_MAX_BROADCAST_MEMORY = "query_max_broadcast_memory";
116116
public static final String QUERY_MAX_TOTAL_MEMORY = "query_max_total_memory";
117117
public static final String QUERY_MAX_TOTAL_MEMORY_PER_NODE = "query_max_total_memory_per_node";
118+
public static final String QUERY_MAX_QUEUED_TIME = "query_max_queued_time";
118119
public static final String QUERY_MAX_EXECUTION_TIME = "query_max_execution_time";
119120
public static final String QUERY_MAX_RUN_TIME = "query_max_run_time";
120121
public static final String RESOURCE_OVERCOMMIT = "resource_overcommit";
@@ -569,6 +570,15 @@ public SystemSessionProperties(
569570
false,
570571
value -> Duration.valueOf((String) value),
571572
Duration::toString),
573+
new PropertyMetadata<>(
574+
QUERY_MAX_QUEUED_TIME,
575+
"Maximum Queued time of a query",
576+
VARCHAR,
577+
Duration.class,
578+
queryManagerConfig.getQueryMaxQueuedTime(),
579+
false,
580+
value -> Duration.valueOf((String) value),
581+
Duration::toString),
572582
new PropertyMetadata<>(
573583
QUERY_MAX_EXECUTION_TIME,
574584
"Maximum execution time of a query",
@@ -2188,6 +2198,11 @@ public static Duration getQueryMaxRunTime(Session session)
21882198
return session.getSystemProperty(QUERY_MAX_RUN_TIME, Duration.class);
21892199
}
21902200

2201+
public static Duration getQueryMaxQueuedTime(Session session)
2202+
{
2203+
return session.getSystemProperty(QUERY_MAX_QUEUED_TIME, Duration.class);
2204+
}
2205+
21912206
public static Duration getQueryMaxExecutionTime(Session session)
21922207
{
21932208
return session.getSystemProperty(QUERY_MAX_EXECUTION_TIME, Duration.class);

presto-main-base/src/main/java/com/facebook/presto/dispatcher/FailedDispatchQuery.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,12 @@ public long getCreateTimeInMillis()
156156
return basicQueryInfo.getQueryStats().getCreateTimeInMillis();
157157
}
158158

159+
@Override
160+
public Duration getQueuedTime()
161+
{
162+
return basicQueryInfo.getQueryStats().getQueuedTime();
163+
}
164+
159165
@Override
160166
public long getExecutionStartTimeInMillis()
161167
{

presto-main-base/src/main/java/com/facebook/presto/execution/AccessControlCheckerExecution.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,12 @@ public long getCreateTimeInMillis()
145145
return stateMachine.getCreateTimeInMillis();
146146
}
147147

148+
@Override
149+
public Duration getQueuedTime()
150+
{
151+
return stateMachine.getQueuedTime();
152+
}
153+
148154
@Override
149155
public long getExecutionStartTimeInMillis()
150156
{

presto-main-base/src/main/java/com/facebook/presto/execution/DataDefinitionExecution.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,12 @@ public long getCreateTimeInMillis()
124124
return stateMachine.getCreateTimeInMillis();
125125
}
126126

127+
@Override
128+
public Duration getQueuedTime()
129+
{
130+
return stateMachine.getQueuedTime();
131+
}
132+
127133
@Override
128134
public long getExecutionStartTimeInMillis()
129135
{

presto-main-base/src/main/java/com/facebook/presto/execution/QueryManagerConfig.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ public class QueryManagerConfig
7474

7575
private String queryExecutionPolicy = "all-at-once";
7676
private Duration queryMaxRunTime = new Duration(100, TimeUnit.DAYS);
77+
private Duration queryMaxQueuedTime = new Duration(100, TimeUnit.DAYS);
7778
private Duration queryMaxExecutionTime = new Duration(100, TimeUnit.DAYS);
7879
private Duration queryMaxCpuTime = new Duration(1_000_000_000, TimeUnit.DAYS);
7980

@@ -431,6 +432,19 @@ public QueryManagerConfig setQueryMaxRunTime(Duration queryMaxRunTime)
431432
return this;
432433
}
433434

435+
@NotNull
436+
public Duration getQueryMaxQueuedTime()
437+
{
438+
return queryMaxQueuedTime;
439+
}
440+
441+
@Config("query.max-queued-time")
442+
public QueryManagerConfig setQueryMaxQueuedTime(Duration queryMaxQueuedTime)
443+
{
444+
this.queryMaxQueuedTime = queryMaxQueuedTime;
445+
return this;
446+
}
447+
434448
@NotNull
435449
public Duration getQueryMaxExecutionTime()
436450
{

presto-main-base/src/main/java/com/facebook/presto/execution/QueryStateMachine.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import com.google.common.util.concurrent.FutureCallback;
5858
import com.google.common.util.concurrent.Futures;
5959
import com.google.common.util.concurrent.ListenableFuture;
60+
import io.airlift.units.Duration;
6061

6162
import javax.annotation.Nullable;
6263
import javax.annotation.concurrent.GuardedBy;
@@ -1025,6 +1026,11 @@ public long getCreateTimeInMillis()
10251026
return queryStateTimer.getCreateTimeInMillis();
10261027
}
10271028

1029+
public Duration getQueuedTime()
1030+
{
1031+
return queryStateTimer.getQueuedTime();
1032+
}
1033+
10281034
public long getExecutionStartTimeInMillis()
10291035
{
10301036
return queryStateTimer.getExecutionStartTimeInMillis();

presto-main-base/src/main/java/com/facebook/presto/execution/QueryTracker.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343

4444
import static com.facebook.presto.SystemSessionProperties.getQueryClientTimeout;
4545
import static com.facebook.presto.SystemSessionProperties.getQueryMaxExecutionTime;
46+
import static com.facebook.presto.SystemSessionProperties.getQueryMaxQueuedTime;
4647
import static com.facebook.presto.SystemSessionProperties.getQueryMaxRunTime;
4748
import static com.facebook.presto.execution.QueryLimit.Source.QUERY;
4849
import static com.facebook.presto.execution.QueryLimit.Source.RESOURCE_GROUP;
@@ -211,22 +212,28 @@ public long getQueriesKilledDueToTooManyTask()
211212
}
212213

213214
/**
214-
* Enforce query max runtime/execution time limits
215+
* Enforce query max runtime/queued/execution time limits
215216
*/
216-
private void enforceTimeLimits()
217+
@VisibleForTesting
218+
void enforceTimeLimits()
217219
{
218220
for (T query : queries.values()) {
219221
if (query.isDone()) {
220222
continue;
221223
}
222224
Duration queryMaxRunTime = getQueryMaxRunTime(query.getSession());
225+
Duration queryMaxQueuedTime = getQueryMaxQueuedTime(query.getSession());
223226
QueryLimit<Duration> queryMaxExecutionTime = getMinimum(
224227
createDurationLimit(getQueryMaxExecutionTime(query.getSession()), QUERY),
225228
query.getResourceGroupQueryLimits()
226229
.flatMap(ResourceGroupQueryLimits::getExecutionTimeLimit)
227230
.map(rgLimit -> createDurationLimit(rgLimit, RESOURCE_GROUP)).orElse(null));
228231
long executionStartTime = query.getExecutionStartTimeInMillis();
229232
long createTimeInMillis = query.getCreateTimeInMillis();
233+
long queuedTimeInMillis = query.getQueuedTime().toMillis();
234+
if (queuedTimeInMillis > queryMaxQueuedTime.toMillis()) {
235+
query.fail(new PrestoException(EXCEEDED_TIME_LIMIT, "Query exceeded maximum queued time limit of " + queryMaxQueuedTime));
236+
}
230237
if (executionStartTime > 0 && (executionStartTime + queryMaxExecutionTime.getLimit().toMillis()) < currentTimeMillis()) {
231238
query.fail(
232239
new PrestoException(EXCEEDED_TIME_LIMIT,
@@ -399,6 +406,8 @@ public interface TrackedQuery
399406

400407
long getCreateTimeInMillis();
401408

409+
Duration getQueuedTime();
410+
402411
long getExecutionStartTimeInMillis();
403412

404413
long getLastHeartbeatInMillis();

presto-main-base/src/main/java/com/facebook/presto/execution/SqlQueryExecution.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,12 @@ public long getCreateTimeInMillis()
341341
return stateMachine.getCreateTimeInMillis();
342342
}
343343

344+
@Override
345+
public Duration getQueuedTime()
346+
{
347+
return stateMachine.getQueuedTime();
348+
}
349+
344350
/**
345351
* For a query that has started executing, returns the timestamp when this query started executing
346352
* Otherwise returns a {@link Optional#empty()}

0 commit comments

Comments
 (0)