Jason Fehr 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 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/22249/4/be/src/scheduling/cluster-membership-mgr.h File be/src/scheduling/cluster-membership-mgr.h: http://gerrit.cloudera.org:8080/#/c/22249/4/be/src/scheduling/cluster-membership-mgr.h@115 PS4, Line 115: std::shared_ptr<ExecutorGroup> all_coordinators; > Should current_membership_lock_ obtained before accessing this? The 'current_membership_lock_' protects the ClusterMembershipMgr::Snapshot 'current_membership_' which is a member of the 'ClusterMembershipMgr' class. This new 'all_coordinators' struct member is part of the ClusterMembershipMgr::Snapshot struct. The ClusterMembershipMgr::Snapshot struct does not protect its own members via locks. That is the responsibility of the code that uses a struct instance. http://gerrit.cloudera.org:8080/#/c/22249/4/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/22249/4/tests/custom_cluster/test_admission_controller.py@1785 PS4, Line 1785: def __assert_systables_query(self, vector, expected_coords=None, : expected_frag_counts=[1, 1, 2], query=ACTIVE_SQL): > Please check custom cluster test that touch fair-scheduler-3-groups.xml and Whether or not the only coordinators request pool is included in executor group determiniation will depend on how the '--expected_executor_group_sets' flag is configured. Using the example from 'test_executor_groups.py' where startup flags include '-expected_executor_group_sets=root.tiny:1,root.small:2,root.large:3', if the only coordinator request pool name started with the prefix "tiny-", "small-", or "large-", then it would be included, otherwise it would be ignored. I will update the docs to call this out. http://gerrit.cloudera.org:8080/#/c/22249/4/tests/custom_cluster/test_admission_controller.py@1795 PS4, Line 1795: exec_options = copy(vector.get_value('exec_option')) : exec_options['request_pool'] = "onlycoords" > After IMPALA-13642, vector is unique per test method. Consider matching the Thanks for the tip. I made the change. http://gerrit.cloudera.org:8080/#/c/22249/4/tests/custom_cluster/test_admission_controller.py@1815 PS4, Line 1815: test_coord_only_pool_happy_path > This test and test_coord_only_pool_non_systables does not crash node in the Done -- 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: 5 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, 29 Jan 2025 18:58:39 +0000 Gerrit-HasComments: Yes
