[VOTE] KIP-73 - Replication Quotas

2016-08-19 Thread Ben Stopford
I’d like to initiate the voting process for KIP-73:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-73+Replication+Quotas 


Ben

Re: [VOTE] KIP-73 - Replication Quotas

2016-08-19 Thread Sriram Subramanian
I think the manual way of setting the throttling value is a good first step
and definitely required when things go bad. We should continue discussing
how we can build more intelligence over this incrementally.

+1 (binding)

On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:

> I’d like to initiate the voting process for KIP-73:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 73+Replication+Quotas  confluence/display/KAFKA/KIP-73+Replication+Quotas>
>
> Ben


Re: [VOTE] KIP-73 - Replication Quotas

2016-08-19 Thread Gwen Shapira
+1 (binding)

On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:
> I’d like to initiate the voting process for KIP-73:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-73+Replication+Quotas 
> 
>
> Ben



-- 
Gwen Shapira
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter | blog


Re: [VOTE] KIP-73 - Replication Quotas

2016-08-22 Thread Jun Rao
Ben,

Thanks for the proposal. +1.

Just a few minor comments below.

1. We have a LeaderOverThrottledRate metric to indicate the amount of
throttling happening in the leader broker. It seems that we should have the
equivalent of that for the follower to indicate the amount of throttling in
the follower, if any.
2. Do we still need the PartitionBytesInRate metric? There is no reference
on how it's going to be used.
3. In the test plan, you mentioned "Then replicas should move at close to
(but no more than) than (the quota dictated rate - the inbound rate).". It
seems that the replicas should always be moved at the quota rate
independent whether there is incoming traffic from the producer?

Jun

On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:

> I’d like to initiate the voting process for KIP-73:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 73+Replication+Quotas  confluence/display/KAFKA/KIP-73+Replication+Quotas>
>
> Ben


Re: [VOTE] KIP-73 - Replication Quotas

2016-08-23 Thread Ismael Juma
Thanks for the KIP, +1 (binding) with a couple of minor suggestions:

//Sample configuration for throttled replicas
{
 "version":1,
 "config": {
  "throttled-replicas":"0-0:0-1:0-2:1-0:1-1:1-2"
 }
}


I think it would be nicer if the "throttled-replicas" value was a JSON
array instead of a String. So:


//Sample configuration for throttled replicas
{
 "version":1,
 "config": {
  "throttled-replicas":["0-0","0-1","0-2","1-0","1-1","1-2"]
 }
}


It does take a little more space, but it's more standard. Do we think
the space savings are worth

coming up with our own encoding?


And about SumReplicaLag, I think ReplicaLagSum sounds a bit better.
Obviously subjective so

if you prefer the current name feel free to keep it.


Ismael


On Mon, Aug 22, 2016 at 11:58 PM, Jun Rao  wrote:

> Ben,
>
> Thanks for the proposal. +1.
>
> Just a few minor comments below.
>
> 1. We have a LeaderOverThrottledRate metric to indicate the amount of
> throttling happening in the leader broker. It seems that we should have the
> equivalent of that for the follower to indicate the amount of throttling in
> the follower, if any.
> 2. Do we still need the PartitionBytesInRate metric? There is no reference
> on how it's going to be used.
> 3. In the test plan, you mentioned "Then replicas should move at close to
> (but no more than) than (the quota dictated rate - the inbound rate).". It
> seems that the replicas should always be moved at the quota rate
> independent whether there is incoming traffic from the producer?
>
> Jun
>
> On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:
>
> > I’d like to initiate the voting process for KIP-73:
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 73+Replication+Quotas  > confluence/display/KAFKA/KIP-73+Replication+Quotas>
> >
> > Ben
>


Re: [VOTE] KIP-73 - Replication Quotas

2016-08-23 Thread Ben Stopford
Thanks for the reviews guys. 

Jun, I’ll removed both LeaderOverThrottledRate as it’s not necessary. 
PartitionBytesInRate could be quite useful, if you want to work out what 
bandwidth a broker requires, based on some arbitrary set of replicas. I might 
keep that one in. 

Ismael - I’m happy with both your points. Will amend. 

B


Ben Stopford
Confluent, http://www.confluent.io 



> On 23 Aug 2016, at 14:41, Ismael Juma  wrote:
> 
> Thanks for the KIP, +1 (binding) with a couple of minor suggestions:
> 
> //Sample configuration for throttled replicas
> {
> "version":1,
> "config": {
>  "throttled-replicas":"0-0:0-1:0-2:1-0:1-1:1-2"
> }
> }
> 
> 
> I think it would be nicer if the "throttled-replicas" value was a JSON
> array instead of a String. So:
> 
> 
> //Sample configuration for throttled replicas
> {
> "version":1,
> "config": {
>  "throttled-replicas":["0-0","0-1","0-2","1-0","1-1","1-2"]
> }
> }
> 
> 
> It does take a little more space, but it's more standard. Do we think
> the space savings are worth
> 
> coming up with our own encoding?
> 
> 
> And about SumReplicaLag, I think ReplicaLagSum sounds a bit better.
> Obviously subjective so
> 
> if you prefer the current name feel free to keep it.
> 
> 
> Ismael
> 
> 
> On Mon, Aug 22, 2016 at 11:58 PM, Jun Rao  wrote:
> 
>> Ben,
>> 
>> Thanks for the proposal. +1.
>> 
>> Just a few minor comments below.
>> 
>> 1. We have a LeaderOverThrottledRate metric to indicate the amount of
>> throttling happening in the leader broker. It seems that we should have the
>> equivalent of that for the follower to indicate the amount of throttling in
>> the follower, if any.
>> 2. Do we still need the PartitionBytesInRate metric? There is no reference
>> on how it's going to be used.
>> 3. In the test plan, you mentioned "Then replicas should move at close to
>> (but no more than) than (the quota dictated rate - the inbound rate).". It
>> seems that the replicas should always be moved at the quota rate
>> independent whether there is incoming traffic from the producer?
>> 
>> Jun
>> 
>> On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:
>> 
>>> I’d like to initiate the voting process for KIP-73:
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>> 73+Replication+Quotas >> confluence/display/KAFKA/KIP-73+Replication+Quotas>
>>> 
>>> Ben
>> 



Re: [VOTE] KIP-73 - Replication Quotas

2016-08-23 Thread Joel Koshy
+1
(sent some very minor edits to you off-thread)

On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:

> I’d like to initiate the voting process for KIP-73:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 73+Replication+Quotas  confluence/display/KAFKA/KIP-73+Replication+Quotas>
>
> Ben


Re: [VOTE] KIP-73 - Replication Quotas

2016-08-23 Thread Ben Stopford
Thanks everyone. It looks like this KIP has now been accepted. 

There is a corresponding PR  for the 
implementation also.

All the best

B


> On 23 Aug 2016, at 22:39, Joel Koshy  wrote:
> 
> +1
> (sent some very minor edits to you off-thread)
> 
> On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:
> 
>> I’d like to initiate the voting process for KIP-73:
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 73+Replication+Quotas > confluence/display/KAFKA/KIP-73+Replication+Quotas>
>> 
>> Ben



Re: [VOTE] KIP-73 - Replication Quotas

2016-08-23 Thread Ismael Juma
For the record, there were 4 binding +1s.

Ismael

On Tue, Aug 23, 2016 at 11:16 PM, Ben Stopford  wrote:

> Thanks everyone. It looks like this KIP has now been accepted.
>
> There is a corresponding PR 
> for the implementation also.
>
> All the best
>
> B
>
>
> > On 23 Aug 2016, at 22:39, Joel Koshy  wrote:
> >
> > +1
> > (sent some very minor edits to you off-thread)
> >
> > On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford  wrote:
> >
> >> I’d like to initiate the voting process for KIP-73:
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> 73+Replication+Quotas  >> confluence/display/KAFKA/KIP-73+Replication+Quotas>
> >>
> >> Ben
>
>


Re: [VOTE] KIP-73 - Replication Quotas

2016-10-19 Thread Jun Rao
Hi,

While testing KIP-73, we found an issue described in
https://issues.apache.org/jira/browse/KAFKA-4313. Basically, when there are
mixed high-volume and low-volume partitions, when replication throttling is
specified, ISRs for those low volume partitions could thrash. KAFKA-4313
fixes this issue by avoiding throttling those replicas in the throttled
replica list that are already in sync. Those in-sync replicas traffic will
still be accounted for the throttled traffic though. Just want to bring
this up since it slightly changes the behavior described in the KIP. If
anyone has concerns on this, please comment on the jira.

Thanks,

Jun

On Tue, Aug 23, 2016 at 3:25 PM, Ismael Juma  wrote:

> For the record, there were 4 binding +1s.
>
> Ismael
>
> On Tue, Aug 23, 2016 at 11:16 PM, Ben Stopford  wrote:
>
> > Thanks everyone. It looks like this KIP has now been accepted.
> >
> > There is a corresponding PR 
> > for the implementation also.
> >
> > All the best
> >
> > B
> >
> >
> > > On 23 Aug 2016, at 22:39, Joel Koshy  wrote:
> > >
> > > +1
> > > (sent some very minor edits to you off-thread)
> > >
> > > On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford 
> wrote:
> > >
> > >> I’d like to initiate the voting process for KIP-73:
> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >> 73+Replication+Quotas  > >> confluence/display/KAFKA/KIP-73+Replication+Quotas>
> > >>
> > >> Ben
> >
> >
>


Re: [VOTE] KIP-73 - Replication Quotas

2016-10-21 Thread Joel Koshy
Thanks for catching that and the fix as well. Makes sense to me.

We should consider adding an "amendments" section to KIPs - perhaps just a
link to KAFKA-4313 would suffice in this case.

Thanks,

Joel

On Wed, Oct 19, 2016 at 7:12 PM, Jun Rao  wrote:

> Hi,
>
> While testing KIP-73, we found an issue described in
> https://issues.apache.org/jira/browse/KAFKA-4313. Basically, when there
> are
> mixed high-volume and low-volume partitions, when replication throttling is
> specified, ISRs for those low volume partitions could thrash. KAFKA-4313
> fixes this issue by avoiding throttling those replicas in the throttled
> replica list that are already in sync. Those in-sync replicas traffic will
> still be accounted for the throttled traffic though. Just want to bring
> this up since it slightly changes the behavior described in the KIP. If
> anyone has concerns on this, please comment on the jira.
>
> Thanks,
>
> Jun
>
> On Tue, Aug 23, 2016 at 3:25 PM, Ismael Juma  wrote:
>
> > For the record, there were 4 binding +1s.
> >
> > Ismael
> >
> > On Tue, Aug 23, 2016 at 11:16 PM, Ben Stopford  wrote:
> >
> > > Thanks everyone. It looks like this KIP has now been accepted.
> > >
> > > There is a corresponding PR  >
> > > for the implementation also.
> > >
> > > All the best
> > >
> > > B
> > >
> > >
> > > > On 23 Aug 2016, at 22:39, Joel Koshy  wrote:
> > > >
> > > > +1
> > > > (sent some very minor edits to you off-thread)
> > > >
> > > > On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford 
> > wrote:
> > > >
> > > >> I’d like to initiate the voting process for KIP-73:
> > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > >> 73+Replication+Quotas  > > >> confluence/display/KAFKA/KIP-73+Replication+Quotas>
> > > >>
> > > >> Ben
> > >
> > >
> >
>


Re: [VOTE] KIP-73 - Replication Quotas

2016-10-24 Thread Jun Rao
Yes, that sounds like a good idea. Updated the wiki for KIP-73.

Thanks,

Jun

On Fri, Oct 21, 2016 at 4:26 PM, Joel Koshy  wrote:

> Thanks for catching that and the fix as well. Makes sense to me.
>
> We should consider adding an "amendments" section to KIPs - perhaps just a
> link to KAFKA-4313 would suffice in this case.
>
> Thanks,
>
> Joel
>
> On Wed, Oct 19, 2016 at 7:12 PM, Jun Rao  wrote:
>
> > Hi,
> >
> > While testing KIP-73, we found an issue described in
> > https://issues.apache.org/jira/browse/KAFKA-4313. Basically, when there
> > are
> > mixed high-volume and low-volume partitions, when replication throttling
> is
> > specified, ISRs for those low volume partitions could thrash. KAFKA-4313
> > fixes this issue by avoiding throttling those replicas in the throttled
> > replica list that are already in sync. Those in-sync replicas traffic
> will
> > still be accounted for the throttled traffic though. Just want to bring
> > this up since it slightly changes the behavior described in the KIP. If
> > anyone has concerns on this, please comment on the jira.
> >
> > Thanks,
> >
> > Jun
> >
> > On Tue, Aug 23, 2016 at 3:25 PM, Ismael Juma  wrote:
> >
> > > For the record, there were 4 binding +1s.
> > >
> > > Ismael
> > >
> > > On Tue, Aug 23, 2016 at 11:16 PM, Ben Stopford 
> wrote:
> > >
> > > > Thanks everyone. It looks like this KIP has now been accepted.
> > > >
> > > > There is a corresponding PR  kafka/pull/1776
> > >
> > > > for the implementation also.
> > > >
> > > > All the best
> > > >
> > > > B
> > > >
> > > >
> > > > > On 23 Aug 2016, at 22:39, Joel Koshy  wrote:
> > > > >
> > > > > +1
> > > > > (sent some very minor edits to you off-thread)
> > > > >
> > > > > On Fri, Aug 19, 2016 at 1:21 AM, Ben Stopford 
> > > wrote:
> > > > >
> > > > >> I’d like to initiate the voting process for KIP-73:
> > > > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > >> 73+Replication+Quotas  > > > >> confluence/display/KAFKA/KIP-73+Replication+Quotas>
> > > > >>
> > > > >> Ben
> > > >
> > > >
> > >
> >
>