Hi Mickael,
Unfortunately, it's not actively being worked on, but I welcome anyone to
come forward. Otherwise it's on my list that I'll eventually get around to.
Currently the logic replicates what was being done historically with the
quota configuration management, and is therefore compatible with the custom
ClientQuotaCallback plugins. To support custom entities, the
ClientQuotaCallback would need to become more generic with respect to
entity types as there's no way to signal an update to a custom type, so
that's the major hangup. Either a new generic quota callback plugin must be
created which can be a superset of the current one, or the current one
needs to be extended.
Sorry I don't have a definitive answer for when it'll be supported, but
it's still on my radar.
Brian
On Wed, Aug 26, 2020 at 6:36 AM Mickael Maison <mickael.mai...@gmail.com>
wrote:
> Hi,
>
> Is anyone actively working on adding support for custom quotas callbacks?
> We should really try to get this in soon as otherwise this KIP is not
> usable in environments with custom quotas. It's unfortunate to have
> the capability on the client-side but no support on brokers.
>
> Thanks
>
>
> On Sat, Jan 25, 2020 at 10:51 AM Colin McCabe <cmcc...@apache.org> wrote:
> >
> > Hi Brian,
> >
> > Thanks for the KIP. I think it looks good and ready for a vote!
> >
> > One nitpick: we try to avoid being too Java-specific when describing
> the protocol. Rather than referencing java.lang.Double, it would be more
> helpful to say something equivalent, like "the float is serialized as a
> 64-bit IEEE 754 value, ordered as big-endian." That way people who are
> developing clients in other languages can more easily understand the
> protocol.
> >
> > best,
> > Colin
> >
> >
> > On Fri, Jan 24, 2020, at 17:18, Brian Byrne wrote:
> > > Thanks for reviewing, Anna,
> > >
> > > In the describe call, the idea is that the API will match the
> QuotaFilters,
> > > which can be specified in two ways for your example (filter is a type
> and a
> > > matching string):
> > >
> > > 1) (USER="user-1") returns both
> > > 2) (USER="user-1", CLIENT_ID="") returns just /config/users/”user-1”
> > >
> > > [For (2), it may be better to permit 'null' to indicate "omitted"
> instead
> > > of the empty string.]
> > >
> > > You're correct for the resolve case, although it may not be
> highlighting
> > > the difference between them. Let's say you had the following:
> > >
> > > /config/users/<default>, value=100
> > >
> > > Then --describe with (USER="user-1") returns empty since there's no
> config
> > > entries for /config/users/”user-1”/*, whereas --resolve with
> > > (USER="user-1") returns (value=100) since the user resolved to the
> default
> > > user config. In other words, --describe is like doing
> > > Admin#describeConfigs, whereas --resolve is like calling
> > > ClientQuotaCallback#quotaLimit, if that helps any.
> > >
> > > Thanks,
> > > Brian
> > >
> > >
> > >
> > > On Fri, Jan 24, 2020 at 3:19 PM Anna Povzner <a...@confluent.io>
> wrote:
> > >
> > > > Hi Brian,
> > > >
> > > > The KIP looks good!
> > > >
> > > > I have one clarification question regarding the distinction between
> > > > describe and resolve API. Suppose I set request quota for
> > > > /config/users/”user-1”/clients/"client-1" to 100 and request quota
> for
> > > > /config/users/”user-1” to 200. Is this correct that
> describeClientQuotas
> > > > called with /config/users/”user-1” would return two entries in the
> > > > response?
> > > >
> > > > -
> > > >
> > > > /config/users/”user-1”/clients/<client-1>, request quota type,
> value =
> > > > 100
> > > >
> > > >
> > > > -
> > > >
> > > > /config/users/”user-1”, request quota type, value = 200
> > > >
> > > >
> > > > While resolve API for entity "/config/users/”user-1” would return
> the quota
> > > > setting specifically for /config/users/”user-1”, which is 200 in
> this case.
> > > >
> > > > Is my understanding correct?
> > > >
> > > > Thanks,
> > > >
> > > > Anna
> > > >
> > > >
> > > > On Fri, Jan 24, 2020 at 11:32 AM Brian Byrne <bby...@confluent.io>
> wrote:
> > > >
> > > > > My apologies, Rajini. My hasty edits omitted a couple spots. I've
> done a
> > > > > more thorough scan and should have cleaned up (1) and (2).
> > > > >
> > > > > For (3), Longs are chosen because that's what the ConfigCommand
> currently
> > > > > uses, and because there's no floating point type in the RPC
> protocol.
> > > > Longs
> > > > > didn't seem to be an issue for bytes-per-second values, and
> > > > > request_percentage is normalized [0-100], but you're right in that
> the
> > > > > extensions might require this.
> > > > >
> > > > > To make Double compatible with the RPC protocol, we'd need to
> serialize
> > > > the
> > > > > value into a String, and then validate the value on the receiving
> end.
> > > > > Would that be acceptable?
> > > > >
> > > > > Thanks,
> > > > > Brian
> > > > >
> > > > > On Fri, Jan 24, 2020 at 11:07 AM Rajini Sivaram <
> rajinisiva...@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Thanks Brian. Looks good.
> > > > > >
> > > > > > Just a few minor points:
> > > > > >
> > > > > > 1) We can remove *public ResolveClientQuotasOptions
> > > > > > setOmitOverriddenValues(boolean omitOverriddenValues); *
> > > > > > 2) Under ClientQuotasCommand, the three items are
> List/Describe/Alter,
> > > > > > rename to match the new naming for operations?
> > > > > > 3) Request quota configs are doubles rather than long. And for
> > > > > > ClientQuotaCallback API, we used doubles everywhere. Wasn't sure
> if we
> > > > > > deliberately chose Longs for this API. if so, we should mention
> why
> > > > under
> > > > > > Rejected Alternatives. We actually use request quotas < 1 in
> > > > integration
> > > > > > tests to ensure we can throttle easily.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Fri, Jan 24, 2020 at 5:28 PM Brian Byrne <bby...@confluent.io
> >
> > > > wrote:
> > > > > >
> > > > > > > Thanks again, Rajini,
> > > > > > >
> > > > > > > Units will have to be implemented on a per-config basis, then.
> I've
> > > > > > removed
> > > > > > > all language reference to units and replaced QuotaKey -> String
> > > > (config
> > > > > > > name). I've also renamed DescribeEffective -> Resolve, and
> replaced
> > > > > > --list
> > > > > > > with --describe, and --describe to --resolve to be consistent
> with
> > > > the
> > > > > > > config command and clear about what functionality is "new".
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Brian
> > > > > > >
> > > > > > > On Fri, Jan 24, 2020 at 2:27 AM Rajini Sivaram <
> > > > > rajinisiva...@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Brian,
> > > > > > > >
> > > > > > > > Thanks for the responses.
> > > > > > > >
> > > > > > > > 4) Yes, agree that it would be simpler to leave units out of
> the
> > > > > > initial
> > > > > > > > design. We currently have units that are interpreted by the
> > > > > > configurable
> > > > > > > > callback. The default callback interprets the value as
> > > > > > > > per-broker-bytes-per-second and per-broker-percentage-cores.
> But
> > > > > > > callbacks
> > > > > > > > using partition-based throughput quotas for example would
> interpret
> > > > > the
> > > > > > > > value as cluster-wide-bytes-per-second. We could update
> callbacks
> > > > to
> > > > > > work
> > > > > > > > with units, but as you said, it may be better to leave it out
> > > > > initially
> > > > > > > and
> > > > > > > > address later.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On Thu, Jan 23, 2020 at 6:29 PM Brian Byrne <
> bby...@confluent.io>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > Thanks Rajini,
> > > > > > > > >
> > > > > > > > > 1) Good catch, fixed.
> > > > > > > > >
> > > > > > > > > 2) You're right. We'd need to extend
> > > > ClientQuotaCallback#quotaLimit
> > > > > > or
> > > > > > > > add
> > > > > > > > > an alternate function. For the sake of an initial
> implementation,
> > > > > I'm
> > > > > > > > going
> > > > > > > > > to remove '--show-overridden', and a subsequent KIP will
> have to
> > > > > > > propose
> > > > > > > > an
> > > > > > > > > extents to ClientQuotaCallback to return more detailed
> > > > information.
> > > > > > > > >
> > > > > > > > > 3) You're correct. I've removed the default.
> > > > > > > > >
> > > > > > > > > 4) The idea of the first iteration is be compatible with
> the
> > > > > existing
> > > > > > > > API,
> > > > > > > > > so no modification to start. The APIs should be kept
> consistent.
> > > > > If a
> > > > > > > > user
> > > > > > > > > wants to add custom functionality, say an entity type,
> they'll
> > > > need
> > > > > > to
> > > > > > > > > update their ConfigEntityType any way, and the quotas APIs
> are
> > > > > meant
> > > > > > to
> > > > > > > > > handle that gracefully by accepting a String which can be
> > > > > propagated.
> > > > > > > > >
> > > > > > > > > The catch is 'units'. Part of the reason for having a
> default
> > > > unit
> > > > > > was
> > > > > > > > for
> > > > > > > > > backwards compatibility, but maybe it's best to leave
> units out
> > > > of
> > > > > > the
> > > > > > > > > initial design. This might lead to adding more
> configuration
> > > > > entries,
> > > > > > > but
> > > > > > > > > it's also the most flexible option. Thoughts?
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Brian
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Thu, Jan 23, 2020 at 4:57 AM Rajini Sivaram <
> > > > > > > rajinisiva...@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Brian,
> > > > > > > > > >
> > > > > > > > > > Thanks for the KIP. Looks good, hope we finally get this
> in!
> > > > > > > > > >
> > > > > > > > > > A few comments:
> > > > > > > > > >
> > > > > > > > > > 1) All the Admin interface methods seem to be using
> method
> > > > names
> > > > > > > > starting
> > > > > > > > > > with upper-case letter, should be lower-case to be follow
> > > > > > > conventions.
> > > > > > > > > > 2) Effective quotas returns not only the actual effective
> > > > quota,
> > > > > > but
> > > > > > > > also
> > > > > > > > > > overridden values. I don't think this works with custom
> quota
> > > > > > > > callbacks.
> > > > > > > > > > 3) KIP says that all quotas are currently
> bytes-per-second and
> > > > we
> > > > > > > will
> > > > > > > > > use
> > > > > > > > > > RATE_BPS as the default. Request quotas are a
> percentage. So
> > > > this
> > > > > > > > doesn't
> > > > > > > > > > quite work. We also need to consider how this works with
> custom
> > > > > > quota
> > > > > > > > > > callbacks. Can custom quota implementations define their
> own
> > > > > units?
> > > > > > > > > > 4) We seem to be defining a new set of quota-related
> classes
> > > > e.g.
> > > > > > for
> > > > > > > > > quota
> > > > > > > > > > types, but we haven't considered what we do with the
> existing
> > > > API
> > > > > > in
> > > > > > > > > > org.apache.kafka.server.quota. Should we keep these
> consistent?
> > > > > Are
> > > > > > > we
> > > > > > > > > > planning to deprecate some of those?
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Regards,
> > > > > > > > > >
> > > > > > > > > > Rajini
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Wed, Jan 22, 2020 at 7:28 PM Brian Byrne <
> > > > bby...@confluent.io
> > > > > >
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Jason,
> > > > > > > > > > >
> > > > > > > > > > > I agree on (1). It was Colin's original suggestion,
> too, but
> > > > he
> > > > > > had
> > > > > > > > > > changed
> > > > > > > > > > > his mind in preference for enums. Strings are the more
> > > > generic
> > > > > > way
> > > > > > > > for
> > > > > > > > > > now,
> > > > > > > > > > > so hopefully Colin can share his thinking when he's
> back. The
> > > > > > > > > QuotaFilter
> > > > > > > > > > > usage was an error, this has been corrected.
> > > > > > > > > > >
> > > > > > > > > > > For (2), the config-centric mode is what we have today
> in
> > > > > > > > > CommandConfig:
> > > > > > > > > > > reading/altering the configuration as it's described.
> The
> > > > > > > > > > > DescribeEffectiveClientQuotas would be resolving the
> various
> > > > > > config
> > > > > > > > > > entries
> > > > > > > > > > > to see what actually applies to a particular entity.
> The
> > > > > examples
> > > > > > > > are a
> > > > > > > > > > > little trivial, but the resolution can become much more
> > > > > > complicated
> > > > > > > > as
> > > > > > > > > > the
> > > > > > > > > > > number of config entries grows.
> > > > > > > > > > >
> > > > > > > > > > > List/describe aren't perfect either. Perhaps
> describe/resolve
> > > > > > are a
> > > > > > > > > > better
> > > > > > > > > > > pair, with DescribeEffectiveClientQuotas ->
> > > > > ResolveClientQuotas?
> > > > > > > > > > >
> > > > > > > > > > > I appreciate the feedback!
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > > Brian
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Jan 21, 2020 at 12:09 PM Jason Gustafson <
> > > > > > > ja...@confluent.io
> > > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Brian,
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for the proposal! I have a couple
> > > > comments/questions:
> > > > > > > > > > > >
> > > > > > > > > > > > 1. I'm having a hard time understanding the point of
> > > > > > > > > > `QuotaEntity.Type`.
> > > > > > > > > > > It
> > > > > > > > > > > > sounds like this might be just for convenience since
> the
> > > > APIs
> > > > > > are
> > > > > > > > > using
> > > > > > > > > > > > string types. If so, I think it's a bit misleading to
> > > > > represent
> > > > > > > it
> > > > > > > > as
> > > > > > > > > > an
> > > > > > > > > > > > enum. In particular, I cannot see how the UNKNOWN
> type
> > > > would
> > > > > be
> > > > > > > > used.
> > > > > > > > > > The
> > > > > > > > > > > > `PrincipalBuilder` plugin allows users to provide
> their own
> > > > > > > > principal
> > > > > > > > > > > type,
> > > > > > > > > > > > so I think the API should be usable even for unknown
> entity
> > > > > > > types.
> > > > > > > > > Note
> > > > > > > > > > > > also that we appear to be relying on this enum in
> > > > > `QuotaFilter`
> > > > > > > > > class.
> > > > > > > > > > I
> > > > > > > > > > > > think that should be changed to just a string?
> > > > > > > > > > > >
> > > > > > > > > > > > 2. It's a little annoying that we have two separate
> APIs to
> > > > > > > > describe
> > > > > > > > > > > client
> > > > > > > > > > > > quotas. The names do not really make it clear which
> API
> > > > > someone
> > > > > > > > > should
> > > > > > > > > > > use.
> > > > > > > > > > > > It might just be a naming problem. In the command
> utility,
> > > > it
> > > > > > > looks
> > > > > > > > > > like
> > > > > > > > > > > > you are using --list and --describe to distinguish
> the two.
> > > > > > > Perhaps
> > > > > > > > > the
> > > > > > > > > > > > APIs can be named similarly: e.g. ListClientQuotas
> and
> > > > > > > > > > > > DescribeClientQuotas. However, looking at the
> examples,
> > > > it's
> > > > > > > still
> > > > > > > > > not
> > > > > > > > > > > very
> > > > > > > > > > > > clear to me why we need both options. Basically I'm
> finding
> > > > > the
> > > > > > > > > > > > "config-centric" mode not very intuitive.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Jason
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > On Fri, Jan 17, 2020 at 2:14 PM Brian Byrne <
> > > > > > bby...@confluent.io
> > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Thanks Colin, I've updated the KIP with the
> relevant
> > > > > changes.
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Fri, Jan 17, 2020 at 10:17 AM Colin McCabe <
> > > > > > > > cmcc...@apache.org>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > I thought about this a little bit more, and
> maybe we
> > > > can
> > > > > > > leave
> > > > > > > > in
> > > > > > > > > > the
> > > > > > > > > > > > > > enums rather than going with strings. But we
> need to
> > > > > have
> > > > > > an
> > > > > > > > > > > "UNKNOWN"
> > > > > > > > > > > > > > value for all the enums, so that if a value that
> the
> > > > > client
> > > > > > > > > doesn't
> > > > > > > > > > > > > > understand is returned, it can get translated to
> that.
> > > > > > This
> > > > > > > is
> > > > > > > > > > what
> > > > > > > > > > > we
> > > > > > > > > > > > > did
> > > > > > > > > > > > > > with the ACLs API, and it worked out well.
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > Done. One thing I omitted here was that the API
> still
> > > > > > > > > accepts/returns
> > > > > > > > > > > > > Strings, since there may be plugins that specify
> their
> > > > own
> > > > > > > > > > types/units.
> > > > > > > > > > > > If
> > > > > > > > > > > > > we'd like to keep it this way, then the UNKNOWN
> may be
> > > > > > > > unnecessary.
> > > > > > > > > > Let
> > > > > > > > > > > > me
> > > > > > > > > > > > > know how you'd feel this is best resolved.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > On balance, I think we should leave in "units."
> It
> > > > could
> > > > > > be
> > > > > > > > > useful
> > > > > > > > > > > for
> > > > > > > > > > > > > > future-proofing.
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > Done. Also added a comment in the
> ClientQuotaCommand to
> > > > > > default
> > > > > > > > to
> > > > > > > > > > > > RATE_BPS
> > > > > > > > > > > > > if no unit is supplied to ease adoption.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Also, since there are other kinds of quotas not
> covered
> > > > > by
> > > > > > > this
> > > > > > > > > > API,
> > > > > > > > > > > we
> > > > > > > > > > > > > > should rename DescribeQuotas ->
> DescribeClientQuotas,
> > > > > > > > AlterQuotas
> > > > > > > > > > ->
> > > > > > > > > > > > > > AlterClientQuotas, etc. etc.
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > Done. Updated command and script name, too.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Maybe QuotaFilter doesn't need a "rule" argument
> to its
> > > > > > > > > constructor
> > > > > > > > > > > > right
> > > > > > > > > > > > > > now. We can just do literal matching for
> everything.
> > > > > > Like I
> > > > > > > > > said
> > > > > > > > > > > > > earlier,
> > > > > > > > > > > > > > I don't think people do a lot of prefixing of
> principal
> > > > > > > names.
> > > > > > > > > > When
> > > > > > > > > > > we
> > > > > > > > > > > > > > added the "prefix matching" stuff for ACLs, it
> was
> > > > mostly
> > > > > > to
> > > > > > > > let
> > > > > > > > > > > people
> > > > > > > > > > > > > do
> > > > > > > > > > > > > > it for topics. Then we made it more generic
> because it
> > > > > was
> > > > > > > > easy
> > > > > > > > > to
> > > > > > > > > > > do
> > > > > > > > > > > > > so.
> > > > > > > > > > > > > > In this case, the API is probably easier to
> understand
> > > > if
> > > > > > we
> > > > > > > > just
> > > > > > > > > > do
> > > > > > > > > > > a
> > > > > > > > > > > > > > literal match. We can always have a follow-on
> KIP to
> > > > add
> > > > > > > > fancier
> > > > > > > > > > > > > filtering
> > > > > > > > > > > > > > if needed.
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > Done.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > For DescribeEffectiveQuotasResult, if you
> request all
> > > > > > > relevant
> > > > > > > > > > > quotas,
> > > > > > > > > > > > it
> > > > > > > > > > > > > > would be nice to see which ones apply and which
> ones
> > > > > don't.
> > > > > > > > > Right
> > > > > > > > > > > now,
> > > > > > > > > > > > > you
> > > > > > > > > > > > > > just get a map, but you don't know which quotas
> are
> > > > > > actually
> > > > > > > in
> > > > > > > > > > > force,
> > > > > > > > > > > > > and
> > > > > > > > > > > > > > which are not relevant but might be in the
> future if a
> > > > > > > > different
> > > > > > > > > > > quota
> > > > > > > > > > > > > gets
> > > > > > > > > > > > > > deleted. One way to do this would be to have two
> > > > maps--
> > > > > > one
> > > > > > > > for
> > > > > > > > > > > > > applicable
> > > > > > > > > > > > > > quotas and one for shadowed quotas.
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > So the way it's specified is that it maps QuotaKey
> ->
> > > > > Value,
> > > > > > > > > however
> > > > > > > > > > > > Value
> > > > > > > > > > > > > is actually defined to have two parts: the entry,
> and a
> > > > > list
> > > > > > of
> > > > > > > > > > > > overridden
> > > > > > > > > > > > > entries (where an entry is the value, along with
> the
> > > > > source).
> > > > > > > > > Perhaps
> > > > > > > > > > > the
> > > > > > > > > > > > > Value is poorly named, or maybe there's a simpler
> > > > structure
> > > > > > to
> > > > > > > be
> > > > > > > > > > had?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Brian
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > On Tue, Jan 14, 2020, at 13:32, Brian Byrne
> wrote:
> > > > > > > > > > > > > > > Hi Colin,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Your feedback is appreciated, thank you.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Tue, Jan 14, 2020 at 11:34 AM Colin McCabe <
> > > > > > > > > > cmcc...@apache.org>
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > This is probably a nitpick, but it would be
> nice to
> > > > > > > specify
> > > > > > > > > > that
> > > > > > > > > > > > this
> > > > > > > > > > > > > > list
> > > > > > > > > > > > > > > > is in order of highest priority to lowest.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Done.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hmm. Maybe --show-overridden or
> > > > --include-overridden
> > > > > > is
> > > > > > > a
> > > > > > > > > > better
> > > > > > > > > > > > > flag
> > > > > > > > > > > > > > > > name?
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Done (--show-overridden).
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I think it would be nice to avoid using
> enums for
> > > > > > > > > > > QuotaEntity#Type,
> > > > > > > > > > > > > > > > QuotaKey#Type, and QuotaFilter#Rule. With
> enums,
> > > > we
> > > > > > have
> > > > > > > > to
> > > > > > > > > > > worry
> > > > > > > > > > > > > > about
> > > > > > > > > > > > > > > > forwards and backwards compatibility
> problems. For
> > > > > > > > example,
> > > > > > > > > > what
> > > > > > > > > > > > do
> > > > > > > > > > > > > > you do
> > > > > > > > > > > > > > > > when you're querying a broker that has a new
> value
> > > > > for
> > > > > > > one
> > > > > > > > of
> > > > > > > > > > > > these,
> > > > > > > > > > > > > > that
> > > > > > > > > > > > > > > > is not in your enum? In the past, we've
> created
> > > > an
> > > > > > > > UNKNOWN
> > > > > > > > > > > value
> > > > > > > > > > > > > for
> > > > > > > > > > > > > > enum
> > > > > > > > > > > > > > > > types to solve this conundrum, but I'm not
> sure the
> > > > > > extra
> > > > > > > > > > > > complexity
> > > > > > > > > > > > > is
> > > > > > > > > > > > > > > > worth it here. We can jut make them strings
> and
> > > > > avoid
> > > > > > > > > worrying
> > > > > > > > > > > > about
> > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > compatibility issues.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Makes sense. Done.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Is QuotaKey#Units really needed? It seems
> like
> > > > > perhaps
> > > > > > > > > > > > QuotaKey#Type
> > > > > > > > > > > > > > > > could imply the units used.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Possibly, maybe. It depends on how much
> structure is
> > > > > > > useful,
> > > > > > > > > > which
> > > > > > > > > > > > > > > influences the implementation in the broker.
> For
> > > > > example,
> > > > > > > for
> > > > > > > > > the
> > > > > > > > > > > > > > existing
> > > > > > > > > > > > > > > (global) bytes-per-second types (e.g. consumer
> byte
> > > > > > rate),
> > > > > > > it
> > > > > > > > > may
> > > > > > > > > > > be
> > > > > > > > > > > > > > useful
> > > > > > > > > > > > > > > to define them on a per-broker BPS basis, and
> in some
> > > > > > > cases,
> > > > > > > > in
> > > > > > > > > > > terms
> > > > > > > > > > > > > of
> > > > > > > > > > > > > > > shares. The question becomes whether it'd be
> better
> > > > to
> > > > > > > have a
> > > > > > > > > > > module
> > > > > > > > > > > > in
> > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > broker that is capable of deducing the
> effective
> > > > quota
> > > > > > > > > > > automatically
> > > > > > > > > > > > > > among
> > > > > > > > > > > > > > > different units for the same quota type, or
> whether
> > > > > each
> > > > > > > > should
> > > > > > > > > > be
> > > > > > > > > > > > > > handled
> > > > > > > > > > > > > > > individually.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Given we don't expect many units, and some
> units may
> > > > be
> > > > > > > > > > > incompatible
> > > > > > > > > > > > > with
> > > > > > > > > > > > > > > others, perhaps it is best to have the unit
> implicit
> > > > in
> > > > > > the
> > > > > > > > > type
> > > > > > > > > > > > > string,
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > be handled by the broker appropriately.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I've updated the KIP to reflect this change,
> which
> > > > > > factors
> > > > > > > > out
> > > > > > > > > > the
> > > > > > > > > > > > > > > QuotaKey. Let me know your thoughts.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > How common is the prefix matching use-case?
> I
> > > > > haven't
> > > > > > > > heard
> > > > > > > > > > > about
> > > > > > > > > > > > > > people
> > > > > > > > > > > > > > > > setting up principal names with a common
> prefix or
> > > > > > > anything
> > > > > > > > > > like
> > > > > > > > > > > > > > that-- is
> > > > > > > > > > > > > > > > that commonly done?
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > It was, in part, for exposition, but would
> handle a
> > > > > case
> > > > > > > > where
> > > > > > > > > > > > > principals
> > > > > > > > > > > > > > > could be prefixed by organization/team name,
> > > > numbered,
> > > > > or
> > > > > > > the
> > > > > > > > > > like.
> > > > > > > > > > > > If
> > > > > > > > > > > > > > you
> > > > > > > > > > > > > > > prefer I remove the rules and just accept a
> pattern,
> > > > > > that's
> > > > > > > > > also
> > > > > > > > > > an
> > > > > > > > > > > > > > option.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I sort of feel like maybe we could have a
> simpler
> > > > API
> > > > > > for
> > > > > > > > > > > > > > describeQuotas
> > > > > > > > > > > > > > > > where it takes a map of quota entity type to
> value,
> > > > > and
> > > > > > > we
> > > > > > > > > do a
> > > > > > > > > > > > > > logical AND
> > > > > > > > > > > > > > > > On that. I'm not sure if there's really a
> reason
> > > > why
> > > > > > it
> > > > > > > > > needs
> > > > > > > > > > to
> > > > > > > > > > > > be
> > > > > > > > > > > > > a
> > > > > > > > > > > > > > > > collection rather than a set, in other
> words...
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > For clarification, are you suggesting an
> interface
> > > > > where
> > > > > > > the
> > > > > > > > > user
> > > > > > > > > > > > might
> > > > > > > > > > > > > > > provide {type=user, name=x} and it would
> return all
> > > > > > > entities
> > > > > > > > > that
> > > > > > > > > > > > > match,
> > > > > > > > > > > > > > > with their resulting quota values? Should I
> scrap
> > > > > pattern
> > > > > > > > > > matching
> > > > > > > > > > > > for
> > > > > > > > > > > > > > now,
> > > > > > > > > > > > > > > since it's a simple extension that can be done
> at a
> > > > > > future
> > > > > > > > > time?
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > Brian
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Wed, Dec 11, 2019, at 15:30, Brian Byrne
> wrote:
> > > > > > > > > > > > > > > > > Hello all,
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > I'm reviving the discussion for adding a
> quotas
> > > > API
> > > > > > to
> > > > > > > > the
> > > > > > > > > > > admin
> > > > > > > > > > > > > > client
> > > > > > > > > > > > > > > > by
> > > > > > > > > > > > > > > > > submitting a new proposal. There are some
> notable
> > > > > > > changes
> > > > > > > > > > from
> > > > > > > > > > > > > > previous
> > > > > > > > > > > > > > > > > attempts, namely a way to deduce the
> effective
> > > > > quota
> > > > > > > for
> > > > > > > > a
> > > > > > > > > > > client
> > > > > > > > > > > > > > > > (entity),
> > > > > > > > > > > > > > > > > a way to query for configured quotas, and
> the
> > > > > concept
> > > > > > > of
> > > > > > > > > > > "units"
> > > > > > > > > > > > on
> > > > > > > > > > > > > > > > quotas,
> > > > > > > > > > > > > > > > > among other minor updates.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Please take a look, and I'll be happy to
> make any
> > > > > > > > > > > clarifications
> > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > modifications in regards to feedback.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-546%3A+Add+quota-specific+APIs+to+the+Admin+Client%2C+redux
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > > Brian
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
