You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "yiguolei (via GitHub)" <gi...@apache.org> on 2023/12/04 12:42:34 UTC
[PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
yiguolei opened a new pull request, #27969:
URL: https://github.com/apache/doris/pull/27969
## Proposed changes
Issue Number: close #xxx
<!--Describe your changes.-->
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840278819
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840384875
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415083016
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,29 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken offerRet = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ offerRet = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
+ if (offerRet != null && !offerRet.enqueueSuccess()) {
String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!offerRet.waitSignal()) {
+ String retMsg = "queue success but wait too long in queue";
Review Comment:
```suggestion
String retMsg = offerRet.getOfferResultDetail();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1842020241
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840947714
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 43.63 seconds
stream load tsv: 572 seconds loaded 74807831229 Bytes, about 124 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.6 seconds inserted 10000000 Rows, about 349K ops/s
storage size: 17167068611 Bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1841932534
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416536717
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,28 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken queueToken = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ queueToken = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
- String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
+ if (queueToken != null && !queueToken.enqueueSuccess()) {
+ String retMsg = "queue failed, reason=" + queueToken.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!queueToken.waitSignal()) {
Review Comment:
```suggestion
if (queueToken!= null && !queueToken.waitSignal()) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416536717
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,28 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken queueToken = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ queueToken = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
- String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
+ if (queueToken != null && !queueToken.enqueueSuccess()) {
+ String retMsg = "queue failed, reason=" + queueToken.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!queueToken.waitSignal()) {
Review Comment:
```suggestion
if (queueToken!= null && !queueToken.waitSignal()) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416536293
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,28 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken queueToken = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ queueToken = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
- String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
+ if (queueToken != null && !queueToken.enqueueSuccess()) {
+ String retMsg = "queue failed, reason=" + queueToken.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!queueToken.waitSignal()) {
Review Comment:
if (queueToken != null && !queueToken.waitSignal())
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1842054572
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit e122241f28a780cd0b64c82c99c7ba3a14a4d66e, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4655 4395 4439 4395
q2 368 156 159 156
q3 1464 1243 1248 1243
q4 1118 926 888 888
q5 3194 3161 3172 3161
q6 243 128 130 128
q7 997 498 494 494
q8 2194 2200 2167 2167
q9 6650 6724 6677 6677
q10 3210 3270 3275 3270
q11 322 211 211 211
q12 359 217 217 217
q13 4633 4225 3785 3785
q14 250 220 211 211
q15 581 527 529 527
q16 452 390 393 390
q17 1006 611 555 555
q18 7441 7004 6992 6992
q19 1577 1483 1364 1364
q20 526 814 342 342
q21 3073 2673 2677 2673
q22 366 289 297 289
Total cold run time: 44679 ms
Total hot run time: 40135 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4372 4331 4366 4331
q2 264 164 173 164
q3 3533 3514 3525 3514
q4 2384 2373 2363 2363
q5 5733 5742 5709 5709
q6 238 122 120 120
q7 2389 1840 1830 1830
q8 3518 3519 3512 3512
q9 9039 9059 8964 8964
q10 3896 4023 3982 3982
q11 494 369 382 369
q12 770 595 595 595
q13 4302 3598 3554 3554
q14 289 258 257 257
q15 581 529 519 519
q16 499 436 451 436
q17 1874 1873 1851 1851
q18 8694 8264 8232 8232
q19 1720 1744 1731 1731
q20 2237 1953 1928 1928
q21 6510 6167 6153 6153
q22 502 404 419 404
Total cold run time: 63838 ms
Total hot run time: 60518 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415092401
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueryQueue.java:
##########
@@ -132,14 +128,29 @@ public QueueOfferToken offer() throws InterruptedException {
}
}
- public void poll() throws InterruptedException {
- queueLock.tryLock(5, TimeUnit.SECONDS);
+ // If the token is acquired and do work success, then call this method to release it.
+ public void returnToken(QueueToken token) throws InterruptedException {
Review Comment:
是不是不是dequeue，dequeue 是出队列的意思，我们是归还token
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840459438
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #27969:
URL: https://github.com/apache/doris/pull/27969
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840987032
run feut
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840141336
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415086779
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueueToken.java:
##########
@@ -0,0 +1,117 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.resource.workloadgroup;
+
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
+// used to mark QueryQueue offer result
+// if offer failed, then need to cancel query
+// and return failed reason to user client
+public class QueueToken {
+
+ enum TokenState {
+ ENQUEUE_FAILED,
+ ENQUEUE_SUCCESS,
+ READY_TO_RUN
+ }
+
+ static AtomicLong tokenIdGenerator = new AtomicLong(0);
+
+ private long tokenId = 0;
+
+ private TokenState tokenState;
+
+ private long waitTimeout = 0;
+
+ private long enqueueTime = 0;
+
+ private String offerResultDetail;
+
+ private final ReentrantLock tokenLock = new ReentrantLock();
+ private final Condition tokenCond = tokenLock.newCondition();
+
+ public QueueToken(TokenState tokenState, long waitTimeout,
+ String offerResultDetail) {
+ this.tokenId = tokenIdGenerator.addAndGet(1);
+ this.tokenState = tokenState;
+ this.waitTimeout = waitTimeout;
+ this.offerResultDetail = offerResultDetail;
+ this.enqueueTime = System.currentTimeMillis();
Review Comment:
enqueueTime is useless, remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1842119896
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1414919079
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueueOfferToken.java:
##########
@@ -17,30 +17,101 @@
package org.apache.doris.resource.workloadgroup;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
// used to mark QueryQueue offer result
// if offer failed, then need to cancel query
// and return failed reason to user client
public class QueueOfferToken {
Review Comment:
```suggestion
public class QueueToken {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840078484
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415089119
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueryQueue.java:
##########
@@ -132,14 +128,29 @@ public QueueOfferToken offer() throws InterruptedException {
}
}
- public void poll() throws InterruptedException {
- queueLock.tryLock(5, TimeUnit.SECONDS);
+ // If the token is acquired and do work success, then call this method to release it.
+ public void returnToken(QueueToken token) throws InterruptedException {
Review Comment:
```suggestion
public void dequeueToken(QueueToken token) throws InterruptedException {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1414921943
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueueOfferToken.java:
##########
@@ -17,30 +17,101 @@
package org.apache.doris.resource.workloadgroup;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
// used to mark QueryQueue offer result
// if offer failed, then need to cancel query
// and return failed reason to user client
public class QueueOfferToken {
- private Boolean offerResult;
+ enum TokenState {
+ ENQUEUE_FAILED,
+ ENQUEUE_SUCCESS,
+ READY_TO_RUN
+ }
+
+ static AtomicLong tokenIdGenerator = new AtomicLong(0);
+
+ private long tokenId = 0;
+
+ private TokenState tokenState;
+
+ private long waitTimeout = 0;
+
+ private long enqueueTime = 0;
Review Comment:
No, I will use it in the future. I will display enqueue time in wg tvf or show process list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415085005
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueryQueue.java:
##########
@@ -91,39 +94,32 @@ public String debugString() {
+ ", currentWaitingQueryNum=" + currentWaitingQueryNum;
}
- public QueueOfferToken offer() throws InterruptedException {
+ public QueueToken getToken() throws InterruptedException {
// to prevent hang
// the lock shouldn't be hold for too long
// we should catch the case when it happens
Review Comment:
remove the comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416536293
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,28 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken queueToken = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ queueToken = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
- String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
+ if (queueToken != null && !queueToken.enqueueSuccess()) {
+ String retMsg = "queue failed, reason=" + queueToken.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!queueToken.waitSignal()) {
Review Comment:
if (queueToken != null && !queueToken.waitSignal())
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840413503
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840499805
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit 4be1455b67323d3c3f37e67682e2ec16fc8cf081, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4687 4434 4424 4424
q2 364 184 157 157
q3 1465 1249 1267 1249
q4 1124 907 869 869
q5 3150 3168 3185 3168
q6 246 125 128 125
q7 1000 496 489 489
q8 2225 2243 2184 2184
q9 6682 6653 6729 6653
q10 3237 3271 3282 3271
q11 329 210 193 193
q12 355 217 216 216
q13 4605 3809 3755 3755
q14 236 214 218 214
q15 578 522 526 522
q16 448 389 381 381
q17 1013 605 553 553
q18 7465 7110 7013 7013
q19 1513 1407 1438 1407
q20 542 329 320 320
q21 3068 2635 2661 2635
q22 354 287 292 287
Total cold run time: 44686 ms
Total hot run time: 40085 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4405 4467 4559 4467
q2 267 159 176 159
q3 3518 3520 3519 3519
q4 2390 2381 2377 2377
q5 5763 5734 5740 5734
q6 237 119 121 119
q7 2375 1845 1887 1845
q8 3520 3524 3533 3524
q9 9044 9044 9049 9044
q10 3918 3991 4000 3991
q11 488 393 383 383
q12 755 624 595 595
q13 4293 3556 3536 3536
q14 278 266 238 238
q15 572 534 514 514
q16 489 456 467 456
q17 1880 1869 1845 1845
q18 8537 8205 8407 8205
q19 1750 1742 1781 1742
q20 2256 1934 1951 1934
q21 6515 6160 6151 6151
q22 497 408 430 408
Total cold run time: 63747 ms
Total hot run time: 60786 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840078514
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415084266
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,29 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken offerRet = null;
Review Comment:
```suggestion
QueueToken queueToken = null;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840954791
<details>
<summary>TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'</summary>
```
Tpch sf100 test result on commit 61c1c83d1e937ec710c34d6539acf880c4367713, data reload: false
run tpch-sf100 query with default conf and session variables
q1 4700 4468 4519 4468
q2 362 150 159 150
q3 1458 1278 1238 1238
q4 1112 940 923 923
q5 3139 3155 3153 3153
q6 248 129 128 128
q7 1002 497 498 497
q8 2161 2220 2172 2172
q9 6652 6776 6662 6662
q10 3244 3365 3273 3273
q11 324 208 200 200
q12 352 203 205 203
q13 4562 4247 3832 3832
q14 240 209 212 209
q15 574 518 523 518
q16 431 379 382 379
q17 1011 578 533 533
q18 7563 7357 6973 6973
q19 1516 1451 1417 1417
q20 552 292 299 292
q21 3090 2666 2653 2653
q22 356 286 293 286
Total cold run time: 44649 ms
Total hot run time: 40159 ms
run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1 4426 4454 4450 4450
q2 264 160 186 160
q3 3535 3508 3508 3508
q4 2379 2362 2357 2357
q5 5718 5745 5751 5745
q6 240 121 123 121
q7 2378 1895 1837 1837
q8 3508 3508 3518 3508
q9 9067 8973 8978 8973
q10 3911 3959 3998 3959
q11 506 372 379 372
q12 756 593 588 588
q13 4312 3580 3562 3562
q14 274 254 249 249
q15 569 519 522 519
q16 501 478 453 453
q17 1859 1852 1832 1832
q18 8640 8276 8404 8276
q19 2581 1797 1723 1723
q20 2261 1951 1926 1926
q21 6475 6256 6108 6108
q22 518 411 411 411
Total cold run time: 64678 ms
Total hot run time: 60637 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415092401
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueryQueue.java:
##########
@@ -132,14 +128,29 @@ public QueueOfferToken offer() throws InterruptedException {
}
}
- public void poll() throws InterruptedException {
- queueLock.tryLock(5, TimeUnit.SECONDS);
+ // If the token is acquired and do work success, then call this method to release it.
+ public void returnToken(QueueToken token) throws InterruptedException {
Review Comment:
是不是不是dequeue，dequeue 是出队列的意思，我们是归还token
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueryQueue.java:
##########
@@ -132,14 +128,29 @@ public QueueOfferToken offer() throws InterruptedException {
}
}
- public void poll() throws InterruptedException {
- queueLock.tryLock(5, TimeUnit.SECONDS);
+ // If the token is acquired and do work success, then call this method to release it.
+ public void returnToken(QueueToken token) throws InterruptedException {
Review Comment:
是不是不是dequeue，dequeue 是出队列的意思，我们是归还token
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1414920257
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueueOfferToken.java:
##########
@@ -17,30 +17,101 @@
package org.apache.doris.resource.workloadgroup;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
// used to mark QueryQueue offer result
// if offer failed, then need to cancel query
// and return failed reason to user client
public class QueueOfferToken {
- private Boolean offerResult;
+ enum TokenState {
+ ENQUEUE_FAILED,
+ ENQUEUE_SUCCESS,
+ READY_TO_RUN
+ }
+
+ static AtomicLong tokenIdGenerator = new AtomicLong(0);
+
+ private long tokenId = 0;
+
+ private TokenState tokenState;
+
+ private long waitTimeout = 0;
+
+ private long enqueueTime = 0;
Review Comment:
unused now, drop it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415089119
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueryQueue.java:
##########
@@ -132,14 +128,29 @@ public QueueOfferToken offer() throws InterruptedException {
}
}
- public void poll() throws InterruptedException {
- queueLock.tryLock(5, TimeUnit.SECONDS);
+ // If the token is acquired and do work success, then call this method to release it.
+ public void returnToken(QueueToken token) throws InterruptedException {
Review Comment:
```suggestion
public void dequeueToken(QueueToken token) throws InterruptedException {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416546462
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,28 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken queueToken = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ queueToken = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
- String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
+ if (queueToken != null && !queueToken.enqueueSuccess()) {
+ String retMsg = "queue failed, reason=" + queueToken.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!queueToken.waitSignal()) {
Review Comment:
It will never be null 啊
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,28 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken queueToken = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ queueToken = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
- String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
+ if (queueToken != null && !queueToken.enqueueSuccess()) {
+ String retMsg = "queue failed, reason=" + queueToken.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!queueToken.waitSignal()) {
Review Comment:
It will never be null 啊
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1841989737
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1415085807
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueryQueue.java:
##########
@@ -132,14 +128,29 @@ public QueueOfferToken offer() throws InterruptedException {
}
}
- public void poll() throws InterruptedException {
- queueLock.tryLock(5, TimeUnit.SECONDS);
+ // If the token is acquired and do work success, then call this method to release it.
+ public void returnToken(QueueToken token) throws InterruptedException {
+ queueLock.lock();
try {
+ // If current token is not in the queue, then do nothing, just return.
+ if (!token.enqueueSuccess()) {
+ return;
+ }
+ // If current token is not in ready to run state, then it is still in the queue
+ // it is not running, just remove it.
+ if (!token.isReadyToRun()) {
+ this.priorityTokenQueue.remove(token);
+ return;
+ }
currentRunningQueryNum--;
Preconditions.checkArgument(currentRunningQueryNum >= 0);
// maybe only when currentWaitingQueryNum != 0 need to signal
if (currentRunningQueryNum < maxConcurrency) {
- queueLockCond.signal();
+ QueueToken nextToken = this.priorityTokenQueue.poll();
+ if (nextToken != null) {
+ ++currentRunningQueryNum;
+ nextToken.signal();
Review Comment:
Add log for signal and wait Exception
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840730796
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416546462
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -607,23 +607,28 @@ public void finalizeQuery() {
private void handleQueryWithRetry(TUniqueId queryId) throws Exception {
// queue query here
syncJournalIfNeeded();
- QueueOfferToken offerRet = null;
+ QueueToken queueToken = null;
QueryQueue queryQueue = null;
if (!parsedStmt.isExplain() && Config.enable_workload_group && Config.enable_query_queue
&& context.getSessionVariable().getEnablePipelineEngine()) {
queryQueue = context.getEnv().getWorkloadGroupMgr().getWorkloadGroupQueryQueue(context);
try {
- offerRet = queryQueue.offer();
+ queueToken = queryQueue.getToken();
} catch (InterruptedException e) {
// this Exception means try lock/await failed, so no need to handle offer result
LOG.error("error happens when offer queue, query id=" + DebugUtil.printId(queryId) + " ", e);
throw new RuntimeException("interrupted Exception happens when queue query");
}
- if (offerRet != null && !offerRet.isOfferSuccess()) {
- String retMsg = "queue failed, reason=" + offerRet.getOfferResultDetail();
+ if (queueToken != null && !queueToken.enqueueSuccess()) {
+ String retMsg = "queue failed, reason=" + queueToken.getOfferResultDetail();
LOG.error("query (id=" + DebugUtil.printId(queryId) + ") " + retMsg);
throw new UserException(retMsg);
}
+ if (!queueToken.waitSignal()) {
Review Comment:
It will never be null 啊
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416599575
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueueToken.java:
##########
@@ -0,0 +1,131 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.resource.workloadgroup;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
+// used to mark QueryQueue offer result
+// if offer failed, then need to cancel query
+// and return failed reason to user client
+public class QueueToken {
+ private static final Logger LOG = LogManager.getLogger(QueueToken.class);
+
+ enum TokenState {
+ ENQUEUE_SUCCESS,
+ READY_TO_RUN
+ }
+
+ static AtomicLong tokenIdGenerator = new AtomicLong(0);
+
+ private long tokenId = 0;
+
+ private TokenState tokenState;
+
+ private long waitTimeout = 0;
+
+ private String offerResultDetail;
+
+ private boolean isTimeout = false;
+
+ private final ReentrantLock tokenLock = new ReentrantLock();
+ private final Condition tokenCond = tokenLock.newCondition();
+
+ public QueueToken(TokenState tokenState, long waitTimeout,
+ String offerResultDetail) {
+ this.tokenId = tokenIdGenerator.addAndGet(1);
+ this.tokenState = tokenState;
+ this.waitTimeout = waitTimeout;
+ this.offerResultDetail = offerResultDetail;
+ }
+
+ public boolean waitSignal() throws InterruptedException {
+ this.tokenLock.lock();
+ try {
+ if (tokenState == TokenState.READY_TO_RUN) {
+ return true;
+ }
+ tokenCond.await(waitTimeout, TimeUnit.MILLISECONDS);
+ // If wait timeout and is steal not ready to run, then return false
+ if (tokenState != TokenState.READY_TO_RUN) {
+ LOG.warn("wait in queue timeout, timeout = {}", waitTimeout);
+ isTimeout = true;
+ return false;
+ } else {
+ return true;
+ }
+ } catch (Throwable t) {
+ LOG.warn("meet execption when wait for signal", t);
+ // If any exception happens, set isTimeout to true and return false
+ // Then the caller will call returnToken to queue normally.
+ offerResultDetail = "meet exeption when wait for signal";
+ isTimeout = true;
+ return false;
+ }finally {
+ this.tokenLock.unlock();
+ }
+ }
+
+ public boolean signal() {
+ this.tokenLock.lock();
+ try {
+ // If current token is not ENQUEUE_SUCCESS, then it maybe has error
+ // not run it any more.
+ if (this.tokenState != TokenState.ENQUEUE_SUCCESS || isTimeout) {
+ return false;
+ }
+ this.tokenState = TokenState.READY_TO_RUN;
+ tokenCond.signal();
Review Comment:
```suggestion
tokenCond.signal();
this.tokenState = TokenState.READY_TO_RUN;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1842050940
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 43.58 seconds
stream load tsv: 558 seconds loaded 74807831229 Bytes, about 127 MB/s
stream load json: 19 seconds loaded 2358488459 Bytes, about 118 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 29.0 seconds inserted 10000000 Rows, about 344K ops/s
storage size: 17162175080 Bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840519936
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.9 seconds
stream load tsv: 565 seconds loaded 74807831229 Bytes, about 126 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 27.9 seconds inserted 10000000 Rows, about 358K ops/s
storage size: 17164009892 Bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "wangbo (via GitHub)" <gi...@apache.org>.
wangbo commented on code in PR #27969:
URL: https://github.com/apache/doris/pull/27969#discussion_r1416599575
##########
fe/fe-core/src/main/java/org/apache/doris/resource/workloadgroup/QueueToken.java:
##########
@@ -0,0 +1,131 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.resource.workloadgroup;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
+// used to mark QueryQueue offer result
+// if offer failed, then need to cancel query
+// and return failed reason to user client
+public class QueueToken {
+ private static final Logger LOG = LogManager.getLogger(QueueToken.class);
+
+ enum TokenState {
+ ENQUEUE_SUCCESS,
+ READY_TO_RUN
+ }
+
+ static AtomicLong tokenIdGenerator = new AtomicLong(0);
+
+ private long tokenId = 0;
+
+ private TokenState tokenState;
+
+ private long waitTimeout = 0;
+
+ private String offerResultDetail;
+
+ private boolean isTimeout = false;
+
+ private final ReentrantLock tokenLock = new ReentrantLock();
+ private final Condition tokenCond = tokenLock.newCondition();
+
+ public QueueToken(TokenState tokenState, long waitTimeout,
+ String offerResultDetail) {
+ this.tokenId = tokenIdGenerator.addAndGet(1);
+ this.tokenState = tokenState;
+ this.waitTimeout = waitTimeout;
+ this.offerResultDetail = offerResultDetail;
+ }
+
+ public boolean waitSignal() throws InterruptedException {
+ this.tokenLock.lock();
+ try {
+ if (tokenState == TokenState.READY_TO_RUN) {
+ return true;
+ }
+ tokenCond.await(waitTimeout, TimeUnit.MILLISECONDS);
+ // If wait timeout and is steal not ready to run, then return false
+ if (tokenState != TokenState.READY_TO_RUN) {
+ LOG.warn("wait in queue timeout, timeout = {}", waitTimeout);
+ isTimeout = true;
+ return false;
+ } else {
+ return true;
+ }
+ } catch (Throwable t) {
+ LOG.warn("meet execption when wait for signal", t);
+ // If any exception happens, set isTimeout to true and return false
+ // Then the caller will call returnToken to queue normally.
+ offerResultDetail = "meet exeption when wait for signal";
+ isTimeout = true;
+ return false;
+ }finally {
+ this.tokenLock.unlock();
+ }
+ }
+
+ public boolean signal() {
+ this.tokenLock.lock();
+ try {
+ // If current token is not ENQUEUE_SUCCESS, then it maybe has error
+ // not run it any more.
+ if (this.tokenState != TokenState.ENQUEUE_SUCCESS || isTimeout) {
+ return false;
+ }
+ this.tokenState = TokenState.READY_TO_RUN;
+ tokenCond.signal();
Review Comment:
```suggestion
tokenCond.signal();
this.tokenState = TokenState.READY_TO_RUN;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [refactor](queryqueue) using a priority queue in query queue in order to implement priority management in the future [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #27969:
URL: https://github.com/apache/doris/pull/27969#issuecomment-1840276117
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org