Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/22422 )
Change subject: IMPALA-13703: Cancel running queries before shutdown deadline ...................................................................... Patch Set 9: (5 comments) http://gerrit.cloudera.org:8080/#/c/22422/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22422/8//COMMIT_MSG@16 PS8, Line 16: cancel any remaining running queries within a configurable timelimit > This is a substantial change to shutdown. Queries that might have completed The idea is to cancel queries only near the end of the graceful shutdown deadline. We set the threshold at 80% of the shutdown deadline, if queries are still running at this point, we allow to take a more aggressive way by canceling them to ensure the queries ends in a graceful way. I’m open to adjusting the 0.2 ratio, picked it because it seemed reasonable, and memory reservation also uses 20% as a line. Added some comments. http://gerrit.cloudera.org:8080/#/c/22422/8/be/src/runtime/query-exec-mgr.cc File be/src/runtime/query-exec-mgr.cc: http://gerrit.cloudera.org:8080/#/c/22422/8/be/src/runtime/query-exec-mgr.cc@259 PS8, Line 259: const int remaining_queue_size = QUERY_EXEC_MGR_MAX_CANCELLATION_QUEUE_SIZE > `remaining_queue_size` should probably be const. Done http://gerrit.cloudera.org:8080/#/c/22422/8/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/22422/8/be/src/service/impala-server.cc@328 PS8, Line 328: "down process. If this duration elapses after the shut down process is started, " > This line wasn't too long, why the change in formatting? Done http://gerrit.cloudera.org:8080/#/c/22422/8/be/src/service/impala-server.cc@499 PS8, Line 499: > Why have this guard? I'd think any value less than the shutdown deadline co I think the idea is to cancel queries only near the end of the graceful shutdown deadline. Set a threshold here to allow a more aggressive way if queries are still running, canceling them is more graceful than shutting down without handling them http://gerrit.cloudera.org:8080/#/c/22422/8/tests/custom_cluster/test_restart_services.py File tests/custom_cluster/test_restart_services.py: http://gerrit.cloudera.org:8080/#/c/22422/8/tests/custom_cluster/test_restart_services.py@1004 PS8, Line 1004: self.execute_query_expect_success(self.client, SHUTDOWN) > Should we check for deadline expired after executors are shutdown and befor Done -- To view, visit http://gerrit.cloudera.org:8080/22422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1cac2e100d329644e21fdceb0b23901b08079130 Gerrit-Change-Number: 22422 Gerrit-PatchSet: 9 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 06 Feb 2025 01:44:11 +0000 Gerrit-HasComments: Yes
