Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22249 )
Change subject: IMPALA-13201: System Table Queries Execute When Admission Queues are Full ...................................................................... Patch Set 24: (4 comments) http://gerrit.cloudera.org:8080/#/c/22249/24/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/22249/24/fe/src/main/java/org/apache/impala/service/Frontend.java@2201 PS24, Line 2201: coord_only_request_pool nit: use camel case. There have been mixed snake vs camel case style in Frontend.java, but I think camel case should be the way to for anything not Thrift related. http://gerrit.cloudera.org:8080/#/c/22249/24/java/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java File java/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java: http://gerrit.cloudera.org:8080/#/c/22249/24/java/yarn-extras/src/main/java/org/apache/impala/yarn/server/resourcemanager/scheduler/fair/AllocationFileLoaderService.java@83 PS24, Line 83: : // Path to XML file containing allocations. : private File allocFile; : : private Listener reloadListener; : nit: cleanup the trailing white spaces. http://gerrit.cloudera.org:8080/#/c/22249/24/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/22249/24/tests/custom_cluster/test_admission_controller.py@1824 PS24, Line 1824: "Frontend did not produce the correct verdict during executor " \ : "group determination" I think it is better to make this verbose. pattern = (r'\n\s+Executor group 1:\n\s+Verdict: Assign to first group ' r'because only coordinators request pool specified') fe_verdict = re.search(pattern, profile) assert fe_verdict, "Can not find {0} pattern in query profile:\n{1}".format(pattern, profile) http://gerrit.cloudera.org:8080/#/c/22249/24/tests/custom_cluster/test_admission_controller.py@1996 PS24, Line 1996: use_exclusive_coordinators=True, Is this starting a new Coordinator or Executor? Same question for L2007. Coordinator usually is in 'coordinator' group. https://github.com/apache/impala/blob/c9e9f86c0536f0991f70613675bbcfbbccc6a324/tests/custom_cluster/test_executor_groups.py#L91 The options in L1993 has --executor_groups pattern for an Executor like this. https://github.com/apache/impala/blob/c9e9f86c0536f0991f70613675bbcfbbccc6a324/tests/custom_cluster/test_executor_groups.py#L146 -- To view, visit http://gerrit.cloudera.org:8080/22249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5e0e64db92bdbf80f8b5bd85d001ffe4c8c9ffda Gerrit-Change-Number: 22249 Gerrit-PatchSet: 24 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 12 Feb 2025 23:01:10 +0000 Gerrit-HasComments: Yes
