kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jun Rao <...@confluent.io>
Subject Re: KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations
Date Tue, 09 Jun 2020 17:28:13 GMT
Hi, David,

Thanks for the explanation. The KIP looks good to me now.

Jun

On Tue, Jun 9, 2020 at 4:27 AM David Jacot <djacot@confluent.io> wrote:

> Hi Jun,
>
> 40. Yes, ThrottleTimeMs is set when the error code is set to QuotaViolated.
> This
> is required to let the client know how long it must wait. This is explained
> in the
> "Handling of new/old clients".
>
> Best,
> David
>
> On Mon, Jun 8, 2020 at 9:29 PM Jun Rao <jun@confluent.io> wrote:
>
> > Hi, David,
> >
> > Thanks for the updated KIP. Another minor comment below.
> >
> > 40. For the new `QUOTA_VIOLATED` error in the response to
> > CreateTopics/CreatePartitions/DeleteTopics, could you clarify
> > whether ThrottleTimeMs is set when the error code is set to
> QUOTA_VIOLATED?
> >
> > Jun
> >
> > On Mon, Jun 8, 2020 at 9:32 AM David Jacot <djacot@confluent.io> wrote:
> >
> > > Hi Jun,
> > >
> > > 30. The rate is accumulated at the partition level. Let me clarify this
> > in
> > > the KIP.
> > >
> > > Best,
> > > David
> > >
> > > On Sat, Jun 6, 2020 at 2:37 AM Anna Povzner <anna@confluent.io> wrote:
> > >
> > > > Hi David,
> > > >
> > > > The KIP looks good to me. I am going to the voting thread...
> > > >
> > > > Hi Jun,
> > > >
> > > > Yes, exactly. That's a separate thing from this KIP, so working on
> the
> > > fix.
> > > >
> > > > Thanks,
> > > > Anna
> > > >
> > > > On Fri, Jun 5, 2020 at 4:36 PM Jun Rao <jun@confluent.io> wrote:
> > > >
> > > > > Hi, Anna,
> > > > >
> > > > > Thanks for the comment. For the problem that you described, perhaps
> > we
> > > > need
> > > > > to make the quota checking and recording more atomic?
> > > > >
> > > > > Hi, David,
> > > > >
> > > > > Thanks for the updated KIP.  Looks good to me now. Just one minor
> > > comment
> > > > > below.
> > > > >
> > > > > 30. controller_mutations_rate: For topic creation and deletion, is
> > the
> > > > rate
> > > > > accumulated at the topic or partition level? It would be useful to
> > make
> > > > it
> > > > > clear in the wiki.
> > > > >
> > > > > Jun
> > > > >
> > > > > On Fri, Jun 5, 2020 at 7:23 AM David Jacot <djacot@confluent.io>
> > > wrote:
> > > > >
> > > > > > Hi Anna and Jun,
> > > > > >
> > > > > > You are right. We should allocate up to the quota for each old
> > > sample.
> > > > > >
> > > > > > I have revamped the Throttling Algorithm section to better
> explain
> > > our
> > > > > > thought process and the token bucket inspiration.
> > > > > >
> > > > > > I have also added a chapter with few guidelines about how to
> define
> > > > > > the quota. There is no magic formula for this but I give few
> > > insights.
> > > > > > I don't have specific numbers that can be used out of the box so
> I
> > > > > > think that it is better to not put any for the time being. We can
> > > > always
> > > > > > complement later on in the documentation.
> > > > > >
> > > > > > Please, take a look and let me know what you think.
> > > > > >
> > > > > > Cheers,
> > > > > > David
> > > > > >
> > > > > > On Fri, Jun 5, 2020 at 8:37 AM Anna Povzner <anna@confluent.io>
> > > wrote:
> > > > > >
> > > > > > > Hi David and Jun,
> > > > > > >
> > > > > > > I dug a bit deeper into the Rate implementation, and wanted to
> > > > confirm
> > > > > > that
> > > > > > > I do believe that the token bucket behavior is better for the
> > > reasons
> > > > > we
> > > > > > > already discussed but wanted to summarize. The main difference
> > > > between
> > > > > > Rate
> > > > > > > and token bucket is that the Rate implementation allows a burst
> > by
> > > > > > > borrowing from the future, whereas a token bucket allows a
> burst
> > by
> > > > > using
> > > > > > > accumulated tokens from the previous idle period. Using
> > accumulated
> > > > > > tokens
> > > > > > > smoothes out the rate measurement in general. Configuring a
> large
> > > > burst
> > > > > > > requires configuring a large quota window, which causes long
> > delays
> > > > for
> > > > > > > bursty workload, due to borrowing credits from the future.
> > Perhaps
> > > it
> > > > > is
> > > > > > > useful to add a summary in the beginning of the Throttling
> > > Algorithm
> > > > > > > section?
> > > > > > >
> > > > > > > In my previous email, I mentioned the issue we observed with
> the
> > > > > > bandwidth
> > > > > > > quota, where a low quota (1MB/s per broker) was limiting
> > bandwidth
> > > > > > visibly
> > > > > > > below the quota. I thought it was strictly the issue with the
> > Rate
> > > > > > > implementation as well, but I found a root cause to be
> different
> > > but
> > > > > > > amplified by the Rate implementation (long throttle delays of
> > > > requests
> > > > > > in a
> > > > > > > burst). I will describe it here for completeness using the
> > > following
> > > > > > > example:
> > > > > > >
> > > > > > >    -
> > > > > > >
> > > > > > >    Quota = 1MB/s, default window size and number of samples
> > > > > > >    -
> > > > > > >
> > > > > > >    Suppose there are 6 connections (maximum 6 outstanding
> > > requests),
> > > > > and
> > > > > > >    each produce request is 5MB. If all requests arrive in a
> > burst,
> > > > the
> > > > > > > last 4
> > > > > > >    requests (20MB over 10MB allowed in a window) may get the
> same
> > > > > > throttle
> > > > > > >    time if they are processed concurrently. We record the rate
> > > under
> > > > > the
> > > > > > > lock,
> > > > > > >    but then calculate throttle time separately after that. So,
> > for
> > > > each
> > > > > > >    request, the observed rate could be 3MB/s, and each request
> > gets
> > > > > > > throttle
> > > > > > >    delay = 20 seconds (instead of 5, 10, 15, 20 respectively).
> > The
> > > > > delay
> > > > > > is
> > > > > > >    longer than the total rate window, which results in lower
> > > > bandwidth
> > > > > > than
> > > > > > >    the quota. Since all requests got the same delay, they will
> > also
> > > > > > arrive
> > > > > > > in
> > > > > > >    a burst, which may also result in longer delay than
> necessary.
> > > It
> > > > > > looks
> > > > > > >    pretty easy to fix, so I will open a separate JIRA for it.
> > This
> > > > can
> > > > > be
> > > > > > >    additionally mitigated by token bucket behavior.
> > > > > > >
> > > > > > >
> > > > > > > For the algorithm "So instead of having one sample equal to 560
> > in
> > > > the
> > > > > > last
> > > > > > > window, we will have 100 samples equal to 5.6.", I agree with
> > Jun.
> > > I
> > > > > > would
> > > > > > > allocate 5 per each old sample that is still in the overall
> > window.
> > > > It
> > > > > > > would be a bit larger granularity than the pure token bucket
> (we
> > > > lose 5
> > > > > > > units / mutation once we move past the sample window), but it
> is
> > > > better
> > > > > > > than the long delay.
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Anna
> > > > > > >
> > > > > > >
> > > > > > > On Thu, Jun 4, 2020 at 6:33 PM Jun Rao <jun@confluent.io>
> wrote:
> > > > > > >
> > > > > > > > Hi, David, Anna,
> > > > > > > >
> > > > > > > > Thanks for the discussion and the updated wiki.
> > > > > > > >
> > > > > > > > 11. If we believe the token bucket behavior is better in
> terms
> > of
> > > > > > > handling
> > > > > > > > the burst behavior, we probably don't need a separate KIP
> since
> > > > it's
> > > > > > just
> > > > > > > > an implementation detail.
> > > > > > > >
> > > > > > > > Regarding "So instead of having one sample equal to 560 in
> the
> > > last
> > > > > > > window,
> > > > > > > > we will have 100 samples equal to 5.6.", I was thinking that
> we
> > > > will
> > > > > > > > allocate 5 to each of the first 99 samples and 65 to the last
> > > > sample.
> > > > > > > Then,
> > > > > > > > 6 new samples have to come before the balance becomes 0
> again.
> > > > > > > Intuitively,
> > > > > > > > we are accumulating credits in each sample. If a usage comes
> > in,
> > > we
> > > > > > first
> > > > > > > > use all existing credits to offset that. If we can't, the
> > > remaining
> > > > > > usage
> > > > > > > > will be recorded in the last sample, which will be offset by
> > > future
> > > > > > > > credits. That seems to match the token bucket behavior the
> > > closest.
> > > > > > > >
> > > > > > > > 20. Could you provide some guidelines on the typical rate
> that
> > an
> > > > > admin
> > > > > > > > should set?
> > > > > > > >
> > > > > > > > Jun
> > > > > > > >
> > > > > > > > On Thu, Jun 4, 2020 at 8:22 AM David Jacot <
> > djacot@confluent.io>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > I just published an updated version of the KIP which
> > includes:
> > > > > > > > > * Using a slightly modified version of our Rate. I have
> tried
> > > to
> > > > > > > > formalize
> > > > > > > > > it based on our discussion. As Anna suggested, we may find
> a
> > > > better
> > > > > > way
> > > > > > > > to
> > > > > > > > > implement it.
> > > > > > > > > * Handling of ValidateOnly as pointed out by Tom.
> > > > > > > > >
> > > > > > > > > Please, check it out and let me know what you think.
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > David
> > > > > > > > >
> > > > > > > > > On Thu, Jun 4, 2020 at 4:57 PM Tom Bentley <
> > > tbentley@redhat.com>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi David,
> > > > > > > > > >
> > > > > > > > > > As a user I might expect the validateOnly option to do
> > > > everything
> > > > > > > > except
> > > > > > > > > > actually make the changes. That interpretation would
> imply
> > > the
> > > > > > quota
> > > > > > > > > should
> > > > > > > > > > be checked, but the check should obviously be side-effect
> > > > free. I
> > > > > > > think
> > > > > > > > > > this interpretation could be useful because it gives the
> > > caller
> > > > > > > either
> > > > > > > > > some
> > > > > > > > > > confidence that they're not going to hit the quota, or
> tell
> > > > them,
> > > > > > via
> > > > > > > > the
> > > > > > > > > > exception, when they can expect the call to work. But for
> > > this
> > > > to
> > > > > > be
> > > > > > > > > useful
> > > > > > > > > > it would require the retry logic to not retry the request
> > > when
> > > > > > > > > validateOnly
> > > > > > > > > > was set.
> > > > > > > > > >
> > > > > > > > > > On the other hand, if validateOnly is really about
> > validating
> > > > > only
> > > > > > > some
> > > > > > > > > > aspects of the request (which maybe is what the name
> > > implies),
> > > > > then
> > > > > > > we
> > > > > > > > > > should clarify in the Javadoc that the quota is not
> > included
> > > in
> > > > > the
> > > > > > > > > > validation.
> > > > > > > > > >
> > > > > > > > > > On balance, I agree with what you're proposing, since the
> > > extra
> > > > > > > utility
> > > > > > > > > of
> > > > > > > > > > including the quota in the validation seems to be not
> worth
> > > the
> > > > > > extra
> > > > > > > > > > complication for the retry.
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > >
> > > > > > > > > > Tom
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Thu, Jun 4, 2020 at 3:32 PM David Jacot <
> > > > djacot@confluent.io>
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Tom,
> > > > > > > > > > >
> > > > > > > > > > > That's a good question. As the validation does not
> create
> > > any
> > > > > > load
> > > > > > > on
> > > > > > > > > the
> > > > > > > > > > > controller, I was planning to do it without checking
> the
> > > > quota
> > > > > at
> > > > > > > > all.
> > > > > > > > > > Does
> > > > > > > > > > > that
> > > > > > > > > > > sound reasonable?
> > > > > > > > > > >
> > > > > > > > > > > Best,
> > > > > > > > > > > David
> > > > > > > > > > >
> > > > > > > > > > > On Thu, Jun 4, 2020 at 4:23 PM David Jacot <
> > > > > djacot@confluent.io>
> > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Jun and Anna,
> > > > > > > > > > > >
> > > > > > > > > > > > Thank you both for your replies.
> > > > > > > > > > > >
> > > > > > > > > > > > Based on our recent discussion, I agree that using a
> > rate
> > > > is
> > > > > > > better
> > > > > > > > > to
> > > > > > > > > > > > remain
> > > > > > > > > > > > consistent with other quotas. As you both suggested,
> it
> > > > seems
> > > > > > > that
> > > > > > > > > > > changing
> > > > > > > > > > > > the way we compute the rate to better handle spiky
> > > > workloads
> > > > > > and
> > > > > > > > > > behave a
> > > > > > > > > > > > bit more similarly to the token bucket algorithm
> makes
> > > > sense
> > > > > > for
> > > > > > > > all
> > > > > > > > > > > > quotas as
> > > > > > > > > > > > well.
> > > > > > > > > > > >
> > > > > > > > > > > > I will update the KIP to reflect this.
> > > > > > > > > > > >
> > > > > > > > > > > > Anna, I think that we can explain this in this KIP.
> We
> > > > can't
> > > > > > just
> > > > > > > > say
> > > > > > > > > > > that
> > > > > > > > > > > > the Rate
> > > > > > > > > > > > will be updated in this KIP. I think that we need to
> > > give a
> > > > > bit
> > > > > > > > more
> > > > > > > > > > > info.
> > > > > > > > > > > >
> > > > > > > > > > > > Best,
> > > > > > > > > > > > David
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, Jun 4, 2020 at 6:31 AM Anna Povzner <
> > > > > anna@confluent.io
> > > > > > >
> > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > >> Hi Jun and David,
> > > > > > > > > > > >>
> > > > > > > > > > > >> Regarding token bucket vs, Rate behavior. We
> recently
> > > > > > observed a
> > > > > > > > > > couple
> > > > > > > > > > > of
> > > > > > > > > > > >> cases where a bursty workload behavior would result
> in
> > > > > > long-ish
> > > > > > > > > pauses
> > > > > > > > > > > in
> > > > > > > > > > > >> between, resulting in lower overall bandwidth than
> the
> > > > > quota.
> > > > > > I
> > > > > > > > will
> > > > > > > > > > > need
> > > > > > > > > > > >> to debug this a bit more to be 100% sure, but it
> does
> > > look
> > > > > > like
> > > > > > > > the
> > > > > > > > > > case
> > > > > > > > > > > >> described by David earlier in this thread. So, I
> agree
> > > > with
> > > > > > Jun
> > > > > > > > -- I
> > > > > > > > > > > think
> > > > > > > > > > > >> we should make all quota rate behavior consistent,
> and
> > > > > > probably
> > > > > > > > > > similar
> > > > > > > > > > > to
> > > > > > > > > > > >> the token bucket one.
> > > > > > > > > > > >>
> > > > > > > > > > > >> Looking at KIP-13, it doesn't describe rate
> > calculation
> > > in
> > > > > > > enough
> > > > > > > > > > > detail,
> > > > > > > > > > > >> but does mention window size. So, we could keep
> > "window
> > > > > size"
> > > > > > > and
> > > > > > > > > > > "number
> > > > > > > > > > > >> of samples" configs and change Rate implementation
> to
> > be
> > > > > more
> > > > > > > > > similar
> > > > > > > > > > to
> > > > > > > > > > > >> token bucket:
> > > > > > > > > > > >> * number of samples define our burst size
> > > > > > > > > > > >> * Change the behavior, which could be described as:
> > If a
> > > > > burst
> > > > > > > > > happens
> > > > > > > > > > > >> after an idle period, the burst would effectively
> > spread
> > > > > > evenly
> > > > > > > > over
> > > > > > > > > > the
> > > > > > > > > > > >> (now - window) time period, where window is (<number
> > of
> > > > > > > samples> -
> > > > > > > > > 1)*
> > > > > > > > > > > >> <window size>. Which basically describes a token
> > bucket,
> > > > > while
> > > > > > > > > keeping
> > > > > > > > > > > the
> > > > > > > > > > > >> current quota configs. I think we can even implement
> > > this
> > > > by
> > > > > > > > > changing
> > > > > > > > > > > the
> > > > > > > > > > > >> way we record the last sample or lastWindowMs.
> > > > > > > > > > > >>
> > > > > > > > > > > >> Jun, if we would be changing Rate calculation
> behavior
> > > in
> > > > > > > > bandwidth
> > > > > > > > > > and
> > > > > > > > > > > >> request quotas, would we need a separate KIP?
> > Shouldn't
> > > > need
> > > > > > to
> > > > > > > if
> > > > > > > > > we
> > > > > > > > > > > >> keep window size and number of samples configs,
> right?
> > > > > > > > > > > >>
> > > > > > > > > > > >> Thanks,
> > > > > > > > > > > >> Anna
> > > > > > > > > > > >>
> > > > > > > > > > > >> On Wed, Jun 3, 2020 at 3:24 PM Jun Rao <
> > > jun@confluent.io>
> > > > > > > wrote:
> > > > > > > > > > > >>
> > > > > > > > > > > >> > Hi, David,
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > Thanks for the reply.
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > 11. To match the behavior in the Token bucket
> > > approach,
> > > > I
> > > > > > was
> > > > > > > > > > thinking
> > > > > > > > > > > >> that
> > > > > > > > > > > >> > requests that don't fit in the previous time
> windows
> > > > will
> > > > > be
> > > > > > > > > > > >> accumulated in
> > > > > > > > > > > >> > the current time window. So, the 60 extra requests
> > > will
> > > > be
> > > > > > > > > > accumulated
> > > > > > > > > > > >> in
> > > > > > > > > > > >> > the latest window. Then, the client also has to
> wait
> > > for
> > > > > 12
> > > > > > > more
> > > > > > > > > > secs
> > > > > > > > > > > >> > before throttling is removed. I agree that this is
> > > > > probably
> > > > > > a
> > > > > > > > > better
> > > > > > > > > > > >> > behavior and it's reasonable to change the
> existing
> > > > > behavior
> > > > > > > to
> > > > > > > > > this
> > > > > > > > > > > >> one.
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > To me, it seems that sample_size * num_windows is
> > the
> > > > same
> > > > > > as
> > > > > > > > max
> > > > > > > > > > > burst
> > > > > > > > > > > >> > balance. The latter seems a bit better to
> configure.
> > > The
> > > > > > thing
> > > > > > > > is
> > > > > > > > > > that
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > existing quota system has already been used in
> > quite a
> > > > few
> > > > > > > > places
> > > > > > > > > > and
> > > > > > > > > > > >> if we
> > > > > > > > > > > >> > want to change the configuration in the future,
> > there
> > > is
> > > > > the
> > > > > > > > > > migration
> > > > > > > > > > > >> > cost. Given that, do you feel it's better to adopt
> > the
> > > > > new
> > > > > > > > token
> > > > > > > > > > > bucket
> > > > > > > > > > > >> > terminology or just adopt the behavior somehow
> into
> > > our
> > > > > > > existing
> > > > > > > > > > > >> system? If
> > > > > > > > > > > >> > it's the former, it would be useful to document
> this
> > > in
> > > > > the
> > > > > > > > > rejected
> > > > > > > > > > > >> > section and add a future plan on migrating
> existing
> > > > quota
> > > > > > > > > > > >> configurations.
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > Jun
> > > > > > > > > > > >> >
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > On Tue, Jun 2, 2020 at 6:55 AM David Jacot <
> > > > > > > djacot@confluent.io
> > > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > > Hi Jun,
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Thanks for your reply.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > 10. I think that both options are likely
> > equivalent
> > > > from
> > > > > > an
> > > > > > > > > > accuracy
> > > > > > > > > > > >> > point
> > > > > > > > > > > >> > > of
> > > > > > > > > > > >> > > view. If we put the implementation aside,
> > > > conceptually,
> > > > > I
> > > > > > am
> > > > > > > > not
> > > > > > > > > > > >> > convinced
> > > > > > > > > > > >> > > by the used based approach because resources
> don't
> > > > have
> > > > > a
> > > > > > > > clear
> > > > > > > > > > > owner
> > > > > > > > > > > >> > > in AK at the moment. A topic can be created by
> > > > > (Principal
> > > > > > A,
> > > > > > > > no
> > > > > > > > > > > client
> > > > > > > > > > > >> > id),
> > > > > > > > > > > >> > > then can be extended by (no principal, Client
> B),
> > > and
> > > > > > > finally
> > > > > > > > > > > deleted
> > > > > > > > > > > >> by
> > > > > > > > > > > >> > > (Principal C, Client C). This does not sound
> right
> > > to
> > > > me
> > > > > > > and I
> > > > > > > > > > fear
> > > > > > > > > > > >> that
> > > > > > > > > > > >> > it
> > > > > > > > > > > >> > > is not going to be easy to grasp for our users.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Regarding the naming, I do agree that we can
> make
> > it
> > > > > more
> > > > > > > > future
> > > > > > > > > > > >> proof.
> > > > > > > > > > > >> > > I propose `controller_mutations_rate`. I think
> > that
> > > > > > > > > > differentiating
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > mutations
> > > > > > > > > > > >> > > from the reads is still a good thing for the
> > future.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > 11. I am not convinced by your proposal for the
> > > > > following
> > > > > > > > > reasons:
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > First, in my toy example, I used 101 windows and
> > 7 *
> > > > 80
> > > > > > > > > requests.
> > > > > > > > > > We
> > > > > > > > > > > >> > could
> > > > > > > > > > > >> > > effectively allocate 5 * 100 requests to the
> > > previous
> > > > > > > windows
> > > > > > > > > > > assuming
> > > > > > > > > > > >> > that
> > > > > > > > > > > >> > > they are empty. What shall we do with the
> > remaining
> > > 60
> > > > > > > > requests?
> > > > > > > > > > > >> Shall we
> > > > > > > > > > > >> > > allocate them to the current window or shall we
> > > divide
> > > > > it
> > > > > > > > among
> > > > > > > > > > all
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > windows?
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Second, I don't think that we can safely change
> > the
> > > > > > behavior
> > > > > > > > of
> > > > > > > > > > all
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > existing
> > > > > > > > > > > >> > > rates used because it actually changes the
> > > computation
> > > > > of
> > > > > > > the
> > > > > > > > > rate
> > > > > > > > > > > as
> > > > > > > > > > > >> > > values
> > > > > > > > > > > >> > > allocated to past windows would expire before
> they
> > > > would
> > > > > > > > today.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Overall, while trying to fit in the current
> rate,
> > we
> > > > are
> > > > > > > going
> > > > > > > > > to
> > > > > > > > > > > >> build a
> > > > > > > > > > > >> > > slightly
> > > > > > > > > > > >> > > different version of the rate which will be even
> > > more
> > > > > > > > confusing
> > > > > > > > > > for
> > > > > > > > > > > >> > users.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Instead, I think that we should embrace the
> notion
> > > of
> > > > > > burst
> > > > > > > as
> > > > > > > > > it
> > > > > > > > > > > >> could
> > > > > > > > > > > >> > > also
> > > > > > > > > > > >> > > be applied to other quotas in the future. Users
> > > don't
> > > > > have
> > > > > > > to
> > > > > > > > > know
> > > > > > > > > > > >> that
> > > > > > > > > > > >> > we
> > > > > > > > > > > >> > > use the Token Bucket or a special rate inside at
> > the
> > > > end
> > > > > > of
> > > > > > > > the
> > > > > > > > > > day.
> > > > > > > > > > > >> It
> > > > > > > > > > > >> > is
> > > > > > > > > > > >> > > an
> > > > > > > > > > > >> > > implementation detail.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Users would be able to define:
> > > > > > > > > > > >> > > - a rate R; and
> > > > > > > > > > > >> > > - a maximum burst B.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > If we change the metrics to be as follow:
> > > > > > > > > > > >> > > - the actual rate;
> > > > > > > > > > > >> > > - the burst balance in %, 0 meaning that the
> user
> > is
> > > > > > > > throttled;
> > > > > > > > > > > >> > > It remains disattach from the algorithm.
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > I personally prefer this over having to define a
> > > rate
> > > > > and
> > > > > > a
> > > > > > > > > number
> > > > > > > > > > > of
> > > > > > > > > > > >> > > windows
> > > > > > > > > > > >> > > while having to understand that the number of
> > > windows
> > > > > > > > implicitly
> > > > > > > > > > > >> defines
> > > > > > > > > > > >> > > the
> > > > > > > > > > > >> > > allowed burst. I think that it is clearer and
> > easier
> > > > to
> > > > > > > grasp.
> > > > > > > > > > Don't
> > > > > > > > > > > >> you?
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > Best,
> > > > > > > > > > > >> > > David
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > On Fri, May 29, 2020 at 6:38 PM Jun Rao <
> > > > > jun@confluent.io
> > > > > > >
> > > > > > > > > wrote:
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> > > > Hi, David, Anna,
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > Thanks for the response. Sorry for the late
> > reply.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > 10. Regarding exposing rate or usage as quota.
> > > Your
> > > > > > > argument
> > > > > > > > > is
> > > > > > > > > > > that
> > > > > > > > > > > >> > > usage
> > > > > > > > > > > >> > > > is not very accurate anyway and is harder to
> > > > > implement.
> > > > > > > So,
> > > > > > > > > > let's
> > > > > > > > > > > >> just
> > > > > > > > > > > >> > > be a
> > > > > > > > > > > >> > > > bit loose and expose rate. I am sort of
> neutral
> > on
> > > > > that.
> > > > > > > (1)
> > > > > > > > > It
> > > > > > > > > > > >> seems
> > > > > > > > > > > >> > to
> > > > > > > > > > > >> > > me
> > > > > > > > > > > >> > > > that overall usage will be relatively more
> > > accurate
> > > > > than
> > > > > > > > rate.
> > > > > > > > > > All
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > > issues that Anna brought up seem to exist for
> > rate
> > > > > too.
> > > > > > > Rate
> > > > > > > > > has
> > > > > > > > > > > the
> > > > > > > > > > > >> > > > additional problem that the cost of each
> request
> > > may
> > > > > not
> > > > > > > be
> > > > > > > > > > > uniform.
> > > > > > > > > > > >> > (2)
> > > > > > > > > > > >> > > In
> > > > > > > > > > > >> > > > terms of implementation, a usage based
> approach
> > > > > requires
> > > > > > > > > > tracking
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > user
> > > > > > > > > > > >> > > > info through the life cycle of an operation.
> > > > However,
> > > > > as
> > > > > > > you
> > > > > > > > > > > >> mentioned,
> > > > > > > > > > > >> > > > things like topic creation can generate
> > additional
> > > > > > > > > > > >> > > > LeaderAndIsr/UpdateMetadata requests. Longer
> > term,
> > > > we
> > > > > > > > probably
> > > > > > > > > > > want
> > > > > > > > > > > >> to
> > > > > > > > > > > >> > > > associate those cost to the user who initiated
> > the
> > > > > > > > operation.
> > > > > > > > > If
> > > > > > > > > > > we
> > > > > > > > > > > >> do
> > > > > > > > > > > >> > > > that, we sort of need to track the user for
> the
> > > full
> > > > > > life
> > > > > > > > > cycle
> > > > > > > > > > of
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > > processing of an operation anyway. (3) If you
> > > prefer
> > > > > > rate
> > > > > > > > > > > strongly,
> > > > > > > > > > > >> I
> > > > > > > > > > > >> > > don't
> > > > > > > > > > > >> > > > have strong objections. However, I do feel
> that
> > > the
> > > > > new
> > > > > > > > quota
> > > > > > > > > > name
> > > > > > > > > > > >> > should
> > > > > > > > > > > >> > > > be able to cover all controller related cost
> > > longer
> > > > > > term.
> > > > > > > > This
> > > > > > > > > > KIP
> > > > > > > > > > > >> > > > currently only covers topic creation/deletion.
> > It
> > > > > would
> > > > > > > not
> > > > > > > > be
> > > > > > > > > > > >> ideal if
> > > > > > > > > > > >> > > in
> > > > > > > > > > > >> > > > the future, we have to add yet another type of
> > > quota
> > > > > for
> > > > > > > > some
> > > > > > > > > > > other
> > > > > > > > > > > >> > > > controller related operations. The quota name
> in
> > > the
> > > > > KIP
> > > > > > > has
> > > > > > > > > > > >> partition
> > > > > > > > > > > >> > > > mutation. In the future, if we allow things
> like
> > > > topic
> > > > > > > > > renaming,
> > > > > > > > > > > it
> > > > > > > > > > > >> may
> > > > > > > > > > > >> > > not
> > > > > > > > > > > >> > > > be related to partition mutation directly and
> it
> > > > would
> > > > > > be
> > > > > > > > > > trickier
> > > > > > > > > > > >> to
> > > > > > > > > > > >> > fit
> > > > > > > > > > > >> > > > those operations in the current quota. So,
> maybe
> > > sth
> > > > > > more
> > > > > > > > > > general
> > > > > > > > > > > >> like
> > > > > > > > > > > >> > > > controller_operations_quota will be more
> future
> > > > proof.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > 11. Regarding the difference between the token
> > > > bucket
> > > > > > > > > algorithm
> > > > > > > > > > > and
> > > > > > > > > > > >> our
> > > > > > > > > > > >> > > > current quota mechanism. That's a good
> > > observation.
> > > > It
> > > > > > > seems
> > > > > > > > > > that
> > > > > > > > > > > we
> > > > > > > > > > > >> > can
> > > > > > > > > > > >> > > > make a slight change to our current quota
> > > mechanism
> > > > to
> > > > > > > > > achieve a
> > > > > > > > > > > >> > similar
> > > > > > > > > > > >> > > > thing. As you said, the issue was that we
> > allocate
> > > > all
> > > > > > 7 *
> > > > > > > > 80
> > > > > > > > > > > >> requests
> > > > > > > > > > > >> > in
> > > > > > > > > > > >> > > > the last 1 sec window in our current
> mechanism.
> > > This
> > > > > is
> > > > > > a
> > > > > > > > bit
> > > > > > > > > > > >> > unintuitive
> > > > > > > > > > > >> > > > since each sec only has a quota capacity of 5.
> > An
> > > > > > > > alternative
> > > > > > > > > > way
> > > > > > > > > > > >> is to
> > > > > > > > > > > >> > > > allocate the 7 * 80 requests to all previous
> > > > windows,
> > > > > > each
> > > > > > > > up
> > > > > > > > > to
> > > > > > > > > > > the
> > > > > > > > > > > >> > > > remaining quota capacity left. So, you will
> fill
> > > the
> > > > > > > > current 1
> > > > > > > > > > sec
> > > > > > > > > > > >> > window
> > > > > > > > > > > >> > > > and all previous ones, each with 5. Then, it
> > seems
> > > > > this
> > > > > > > will
> > > > > > > > > > give
> > > > > > > > > > > >> the
> > > > > > > > > > > >> > > same
> > > > > > > > > > > >> > > > behavior as token bucket? The reason that I
> keep
> > > > > asking
> > > > > > > this
> > > > > > > > > is
> > > > > > > > > > > that
> > > > > > > > > > > >> > from
> > > > > > > > > > > >> > > > an operational perspective, it's simpler if
> all
> > > > types
> > > > > of
> > > > > > > > > quotas
> > > > > > > > > > > >> work in
> > > > > > > > > > > >> > > the
> > > > > > > > > > > >> > > > same way.
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > Jun
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > On Tue, May 26, 2020 at 9:52 AM David Jacot <
> > > > > > > > > > djacot@confluent.io>
> > > > > > > > > > > >> > wrote:
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > > > > Hi folks,
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > I have updated the KIP. As mentioned by
> Jun, I
> > > > have
> > > > > > made
> > > > > > > > the
> > > > > > > > > > > >> > > > > quota per principal/clientid similarly to
> the
> > > > other
> > > > > > > > quotas.
> > > > > > > > > I
> > > > > > > > > > > have
> > > > > > > > > > > >> > > > > also explained how this will work in
> > conjunction
> > > > > with
> > > > > > > the
> > > > > > > > > auto
> > > > > > > > > > > >> > > > > topics creation.
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > Please, take a look at it. I plan to call a
> > vote
> > > > for
> > > > > > it
> > > > > > > in
> > > > > > > > > the
> > > > > > > > > > > >> next
> > > > > > > > > > > >> > few
> > > > > > > > > > > >> > > > > days if there are no comments in this
> thread.
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > Best,
> > > > > > > > > > > >> > > > > David
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > On Wed, May 13, 2020 at 10:57 AM Tom
> Bentley <
> > > > > > > > > > > tbentley@redhat.com
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > > > wrote:
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > > > > Hi David,
> > > > > > > > > > > >> > > > > >
> > > > > > > > > > > >> > > > > > Thanks for the explanation and
> confirmation
> > > that
> > > > > > > > evolving
> > > > > > > > > > the
> > > > > > > > > > > >> APIs
> > > > > > > > > > > >> > is
> > > > > > > > > > > >> > > > not
> > > > > > > > > > > >> > > > > > off the table in the longer term.
> > > > > > > > > > > >> > > > > >
> > > > > > > > > > > >> > > > > > Kind regards,
> > > > > > > > > > > >> > > > > >
> > > > > > > > > > > >> > > > > > Tom
> > > > > > > > > > > >> > > > > >
> > > > > > > > > > > >> > > > >
> > > > > > > > > > > >> > > >
> > > > > > > > > > > >> > >
> > > > > > > > > > > >> >
> > > > > > > > > > > >>
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message