Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19214 )
Change subject: WIP IMPALA-7969: Always admit trivial queries immediately ...................................................................... Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG@10 PS1, Line 10: immediate admission Do you know if these trivial queries consume some global resource we can run out of? One small concern I have about these queries is that though we compute them immediately, the client may not fetch them, so actually they can linger on for a long time. If they consume some resource that can block the non-trivial queries from being admitted,then it may be unfair to let them forward unchecked. Probably a limit on the maximum number of parallel trivial queries could make this safer. http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG@14 PS1, Line 14: UNION This could be extended with the union being all const. http://gerrit.cloudera.org:8080/#/c/19214/1//COMMIT_MSG@18 PS1, Line 18: - select * from table limit 0; is this also a valid example? select 1 union all select 2 If yes, then it would be probably useful to have limit for the max number of rows returned as one can create such a query with thousands of union all clauses. http://gerrit.cloudera.org:8080/#/c/19214/1/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/19214/1/be/src/scheduling/admission-controller.cc@1889 PS1, Line 1889: if (state->GetIsTrivialQuery()) { : VLOG_QUERY << "Admiited by trivial query policy."; : queue_node->admitted_schedule = std::move(group_state.state); : return true; : } This will always add trivial queries to the first executor group. Can this cause problems or we ensure that trivial queries are coordinator only and locks 0 resources from the executor group? http://gerrit.cloudera.org:8080/#/c/19214/1/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/1/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@43 PS1, Line 43: if (planRoot.numNodes() != 1) return false; This has to be 1 if it is a PlanRootSink, right? So this could be a Precondition. http://gerrit.cloudera.org:8080/#/c/19214/1/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@44 PS1, Line 44: String displayLabel = planRoot.getDisplayLabel(); : if (displayLabel == null) return false; : if (!displayLabel.equals("00:EMPTYSET") && !displayLabel.equals("00:UNION")) { : return false; : } Can the root node have child nodes? My guess is no, as empty set has no children and the union should be only trivial if it an all const union. http://gerrit.cloudera.org:8080/#/c/19214/1/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@60 PS1, Line 60: sinkRoot.collectExprs(exprs); Will this detect a sleep in the source nodes e.g. select 1 union all select sleep(1000)? http://gerrit.cloudera.org:8080/#/c/19214/1/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/19214/1/tests/custom_cluster/test_admission_controller.py@1202 PS1, Line 1202: "Admission result: Admitted immediately It would be nice to able to check somehow whether a query was handled as trivial, for example by adding a line to the profile like "Query handled as trivial." http://gerrit.cloudera.org:8080/#/c/19214/1/tests/custom_cluster/test_admission_controller.py@1208 PS1, Line 1208: select 1,3 Can you add a test for a more complex constant query, e.g select 1 union all select 3? -- 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: 1 Gerrit-Owner: Yida Wu <wydbaggio...@gmail.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Wed, 09 Nov 2022 12:50:29 +0000 Gerrit-HasComments: Yes
