Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/21616 )
Change subject: IMPALA-12345: Add user quotas to Admission Control ...................................................................... Patch Set 9: (5 comments) Thanks for the review http://gerrit.cloudera.org:8080/#/c/21616/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21616/8//COMMIT_MSG@37 PS8, Line 37: : Two new elements ‘userQueryLimit’ and ‘groupQueryLimit’ can be added to : the fair-scheduler.xml file. These elements can be placed on the root : configuration, which applies to all pools, or the pool configuration. : The ‘userQueryLimit’ element has 2 child elements: "user" and "limit". : The 'user' element contains the short names of users, and can be : repeated, or have the value "*" for a wildcard name which matches all : users. The ‘groupQueryLimit’ element has 2 child eleme > I recommend naming "totalCount" instead of "limit". Changed to "totalCount", thanks. As to future extensions, maybe? How easy to do this? Not to bad I think http://gerrit.cloudera.org:8080/#/c/21616/8/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/21616/8/be/src/scheduling/admission-controller.h@1152 PS8, Line 1152: /// Returns true if there are enough available slots on all executors in the schedule to : /// fit the query schedule. The number of slots per executo > Can user, was_queued, and track_per_user grouped into single optional struc Done http://gerrit.cloudera.org:8080/#/c/21616/8/be/src/scheduling/admission-controller.h@1277 PS8, Line 1277: static bool HasQuotaConfig(const TPoolConfig& pool_cfg); > Add the same comment as HasSufficientGroupQuota Done http://gerrit.cloudera.org:8080/#/c/21616/8/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/21616/8/be/src/scheduling/admission-controller.cc@747 PS8, Line 747: if (per_user_tracking.track_per_user && !per_user_tracking.was_queued) { : // If the query was not previously queued then track the user counts. : IncrementPerUser(per_user_tracking.user); : } > Why there is a per-user tracking but no per-group tracking? All the limits serve to control the count of queries that a user can run. The rules controlling this are expressed in terms of user or groups. I updated the commit message ot be clearer and I will try to make this clear in future user documentation http://gerrit.cloudera.org:8080/#/c/21616/3/fe/src/test/resources/fair-scheduler-test.xml File fe/src/test/resources/fair-scheduler-test.xml: http://gerrit.cloudera.org:8080/#/c/21616/3/fe/src/test/resources/fair-scheduler-test.xml@4 PS3, Line 4: <userQueryLimit> > Thanks. I checked Impala docs and indeed find following sentence: Ack -- To view, visit http://gerrit.cloudera.org:8080/21616 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c33f3f2427db57fb9b6c593a4b22d5029549b41 Gerrit-Change-Number: 21616 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Thu, 26 Sep 2024 18:24:38 +0000 Gerrit-HasComments: Yes
