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 3: (9 comments) http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@340 PS2, Line 340: ss << " local_host(local_mem_admitted=" << PrintBytes(local_mem_admitted_) << ", "; > local_trivial_running_ could be added to the debug string Done http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@784 PS2, Line 784: pools_for_updates_.insert(running_query.reque > Is this log line intentional? Thanks pointing it out. Removed. http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@1914 PS2, Line 1914: << " max_mem=" << PrintBytes(max_mem) > Maybe add state->GetIsTrivialQuery() to the log? Done http://gerrit.cloudera.org:8080/#/c/19214/2/be/src/scheduling/admission-controller.cc@1921 PS2, Line 1921: tor grou > typo Done http://gerrit.cloudera.org:8080/#/c/19214/2/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/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@22 PS2, Line 22: > nit: usually there is a blank line betwwen java. and org. imports Done http://gerrit.cloudera.org:8080/#/c/19214/2/fe/src/main/java/org/apache/impala/planner/TrivialQueryChecker.java@82 PS2, Line 82: * Check whether the query meets the special requirements of a trivial query. > nit: we usually add braces to the block when breaking a line Done http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_admission_controller.py@1216 PS2, Line 1216: "select * from functional_parquet.alltypes limit 1", False) > Can you check what happens when the sleep is after an empty set? e.g. Added the testcases. Thanks, I think I understand the issue now, changed a way to detect the sleep function in TrivialQueryChecker.java, not sure if there is any other case, but it is working with the testcases now. http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_session_expiration.py File tests/custom_cluster/test_session_expiration.py: http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_session_expiration.py@107 PS2, Line 107: # trivial query. > I think the test would be clearer with keeping select 1, but setting enable Done http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_shell_interactive.py File tests/custom_cluster/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/19214/2/tests/custom_cluster/test_shell_interactive.py@51 PS2, Line 51: roc.sendline("set live_summary=t > I think the test would be clearer with keeping select 1, but setting enable Done -- 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: 3 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: Wed, 04 Jan 2023 17:47:36 +0000 Gerrit-HasComments: Yes
