Yida Wu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19214 )

Change subject: IMPALA-7969: Always admit trivial queries immediately
......................................................................


Patch Set 5:

(3 comments)

Added a testcase for the multi-threaded case.

http://gerrit.cloudera.org:8080/#/c/19214/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19214/4//COMMIT_MSG@29
PS4, Line 29: Also, we restrict the parallelism of execution of the trivial
            : query, each resource pool can execute no more than three trivial
            : queries at the same time. If the maximum parallelism is reached,
            : the admission controller would try to admit the trivial query
            : via normal process.
> What happens when you have multiple coordinators? Since the trivial queries
Added comments. Discussed with Abhishek, if working with a global admission 
controller, no matter the coordinator's number, the max concurrent trivial 
query is limited to three per pool, but if there is no global admission 
controller, each coordinator would admit the query based on local variable, so 
it could be X*N trivial queries per resource pool. Maybe we can keep it this 
way in this patch to keep it simple, as long as the trivial queries can be 
restricted to a reasonable parallelism.


http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java
File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java:

http://gerrit.cloudera.org:8080/#/c/19214/3/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@66
PS3, Line 66: HasFunctionSleep
> Hmm, I meant that a new predicate could be created here that filters specif
I added a function in Expr.java, should be simpler now.


http://gerrit.cloudera.org:8080/#/c/19214/4/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java
File fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java:

http://gerrit.cloudera.org:8080/#/c/19214/4/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@71
PS4, Line 71: IS_FN_SLEEP, slee
> This looks strange, as sleep is not an aggregate function
The function was added long ago back to 2014 in 
https://github.com/apache/impala/commit/335c46a2060f4e76f9a689a1a285af7a024353cf,
 and it is no longer used in the code as far as I can see, maybe at that time 
we only support aggregate function. But the implementation is actually to find 
out all the build-in functions.



--
To view, visit http://gerrit.cloudera.org:8080/19214
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a729764e3055d7eb11900c96c82ff53eb261f91
Gerrit-Change-Number: 19214
Gerrit-PatchSet: 5
Gerrit-Owner: Yida Wu <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Tue, 10 Jan 2023 00:51:02 +0000
Gerrit-HasComments: Yes

Reply via email to