Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12852 )
Change subject: [maintenance] Add privilege maintenance thread pool for privilege tables and tablets ...................................................................... Patch Set 1: A few questions about the design/feature: It seems this patch implements a single "privileged" class for tables. Is this flexible enough for all scenarios? I can think of a couple of alternatives: 1) a "boost" which could be applied as a multiplier on top of the op scores for ops in a given tablet. This isn't a guaranteed priority, but can help to make sure that the high-priority tables get prioritized more quickly than others. 2) a more general priority scheme: I think forcing tables into a "normal/high priority" distinction is difficult to manage in a multitenant cluster. I'm afraid this will start getting used, and then at some point someone's going to say "oh, we need a SUPER-HIGH class", etc. Is it worth considering integer priorities? 3) some kind of fair-sharing or budgeting mechanism for maintenance ops across tablets. In other words, in a multitenant use case, we may want to ensure that each tenant gets a fair share of the MM thread pool's resources (perhaps allowing some kind of multiplier/allocation for tenants that are more important). This solves the use case of a single high-priority tenant, but also the more general use case where everyone is equal priority and we don't want a single bad actor to hurt things. That said, another potential issue with MM op prioritization is that flush operations are responsible for keeping memory resources on the tablet server under control. I'm afraid that op prioritization may cause priority inversion: we may choose compactions of high-priority tablets ahead of memory-pressure-reducing flushes of low priority tablets. If the server is under memory pressure, maybe we should ignore priority and flush based on the normal memory-based prioritization? The last question I have is about the use of gflags for this purpose. It seems like this should probably be a table property settable via the API at runtime, rather than a gflag. For example, we could use the 'extra config' functionality provided by https://gerrit.cloudera.org/c/12468/ -- To view, visit http://gerrit.cloudera.org:8080/12852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ea3b73505157678a8fb551656123b64e6bfb304 Gerrit-Change-Number: 12852 Gerrit-PatchSet: 1 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Tue, 26 Mar 2019 04:15:40 +0000 Gerrit-HasComments: No
