Re: [DISCUSS] KIP-504 - Add new Java Authorizer Interface

2019-08-12 Thread David Jacot
Hi Rajini,

Thank you for the KIP. Overall, it looks good to me. I have few
questions/suggestions:

1. It is hard to grasp what `Action#count` is for. I guess I understand
where you want to go with it but it took me a while to figure it out.
Perhaps, we could come up with a better name than `count`?

2. I had a hard time trying to understand the `AuthorizationMode` concept,
especially wrt. the OPTIONAL one. My understanding is that an ACL is either
defined or not. Could you elaborate a bit more on that?

Thanks,
David

On Fri, Aug 9, 2019 at 10:26 PM Don Bosco Durai  wrote:

> Hi Rajini
>
> 3.2 - This makes sense. Thanks for clarifying.
>
> Rest looks fine. Once the implementations are done, it will be more clear
> on the different values RequestType and Mode.
>
> Thanks
>
> Bosco
>
>
> On 8/9/19, 5:08 AM, "Rajini Sivaram"  wrote:
>
> Hi Don,
>
> Thanks for the suggestions. A few responses below:
>
> 3.1 Can rename and improve docs if we keep this. Let's finish the
> discussion on Colin's suggestions regarding this first.
> 3.2 No, I was thinking of some requests that have an old way of
> authorizing
> and a new way where we have retained the old way for backward
> compatibility. One example is Cluster:Create permission to create
> topics.
> We have replaced this with fine-grained topic create access using
> Topic:Create
> for topic patterns. But we still check if user has Cluster:Create
> first. If
> Denied, the deny is ignored and we check Topic:Create. We dont want to
> log
> DENY for Cluster:Create at INFO level for this, since this is not a
> mandatory ACL for creating topics. We will get appropriate logs from
> the
> subsequent Topic:Create in this case.
> 3.3 They are not quite the same. FILTER implies that user actually
> requested access to and performed some operation on the filtered
> resources.
> LIST_AUTHORZED did not result in any actual access. User simply wanted
> to
> know what they are allowed to access.
> 3.4 Request types are Produce, JoinGroup, OffsetCommit etc. So that is
> different from authorization mode, operation etc.
>
>
> On Thu, Aug 8, 2019 at 11:36 PM Don Bosco Durai 
> wrote:
>
> > Hi Rajini
> >
> > Thanks for clarifying. This is very helpful...
> >
> > #1 - On the Ranger side, we should be able to handle multiple
> requests at
> > the same time. I was just not sure how much processing overhead will
> be
> > there on the Broker side to split and then consolidate the results.
> If it
> > is negligible,  then this is the better way. It will make it future
> proof.
> > #2 -  I agree, having a single "start" call makes it cleaner. The
> > Authorization implementation will only have to do the initialization
> only
> > once.
> > #3.1 - Thanks for the clarification. I think it makes sense to have
> this.
> > The term "Mode" might not be explicit enough. Essentially it seems
> you want
> > the Authorizer to know the Intent/Purpose of the authorize call and
> let the
> > Authorizer decide what to log as Audit event. Changing the
> class/field name
> > or giving more documentation will do.
> > #3.2 - Regarding the option "OPTIONAL". Are you thinking from
> chaining
> > multiple Authorizer? If so,  I am not sure whether the Broker would
> have
> > enough information to make that decision. I feel the Authorizer will
> be the
> > one who would have that knowledge. E.g. in Ranger we have explicit
> deny,
> > which means no matter what, the request should be denied for the
> user/group
> > or condition. So if you are thinking of chaining Authorizers, then
> > responses should have the third state, e.g. "DENIED_FINAL", in which
> case
> > if there is an Authorization chain, it will be stop and the request
> will be
> > denied and if it is just denied, then you might fall back to next
> > authorizer. If we don't have chaining of Authorizing in mind, then we
> > should reconsider OPTIONAL for now. Or clarify under which scenario
> > OPTIONAL will be called.
> > #3.3 Regarding, FILTER v/s LIST_AUTHORIZED, isn't LIST_AUTHORIZED a
> > special case for "FILTER"?
> > #3.4 KafkaRequestContext. requestType() v/s Action.
> authorizationMode. I
> > am not sure about the overlap or ambiguity.
> > #4 - Cool. This is good, it will be less stress on the Authorizer.
> Ranger
> > already supports the "count" concept and also has batching
> capability to
> > aggregate similar requests to reduce the number of audit logs to
> write. We
> > should be able to pass this through.
> > #5 - Assuming if the object instance is going out of scope, we
> should be
> > fine. Not a super important ask. Ranger is already catching KILL
> signal for
> > clean up.
> >
> > Thanks again. These are good enhancements. Appreciate your efforts
> here.
> >
> > Bosco
> >
> >
>  

Re: [DISCUSS] KIP-504 - Add new Java Authorizer Interface

2019-08-13 Thread David Jacot
Hi Rajini,

Thank you for the update! It looks good to me. There is a typo in the
`AuditFlag` enum: `MANDATORY_AUTHOEIZE` -> `MANDATORY_AUTHORIZE`.

Regards,
David

On Mon, Aug 12, 2019 at 2:54 PM Rajini Sivaram 
wrote:

> Hi David,
>
> Thanks for reviewing the KIP! Since questions about `authorization mode`
> and `count` have come up multiple times, I have renamed both.
>
> 1) Renamed `count` to `resourceReferenceCount`. It is the number of times
> the resource being authorized is referenced within the request.
>
> 2) Renamed `AuthorizationMode` to `AuditFlag`. It is provided to improve
> audit logging in the authorizer. The enum values have javadoc which
> indicate how the authorization result is used in each of the modes to
> enable authorizers to log audit messages at the right severity level.
>
> Regards,
>
> Rajini
>
> On Mon, Aug 12, 2019 at 5:54 PM David Jacot  wrote:
>
> > Hi Rajini,
> >
> > Thank you for the KIP. Overall, it looks good to me. I have few
> > questions/suggestions:
> >
> > 1. It is hard to grasp what `Action#count` is for. I guess I understand
> > where you want to go with it but it took me a while to figure it out.
> > Perhaps, we could come up with a better name than `count`?
> >
> > 2. I had a hard time trying to understand the `AuthorizationMode`
> concept,
> > especially wrt. the OPTIONAL one. My understanding is that an ACL is
> either
> > defined or not. Could you elaborate a bit more on that?
> >
> > Thanks,
> > David
> >
> > On Fri, Aug 9, 2019 at 10:26 PM Don Bosco Durai 
> wrote:
> >
> > > Hi Rajini
> > >
> > > 3.2 - This makes sense. Thanks for clarifying.
> > >
> > > Rest looks fine. Once the implementations are done, it will be more
> clear
> > > on the different values RequestType and Mode.
> > >
> > > Thanks
> > >
> > > Bosco
> > >
> > >
> > > On 8/9/19, 5:08 AM, "Rajini Sivaram"  wrote:
> > >
> > > Hi Don,
> > >
> > > Thanks for the suggestions. A few responses below:
> > >
> > > 3.1 Can rename and improve docs if we keep this. Let's finish the
> > > discussion on Colin's suggestions regarding this first.
> > > 3.2 No, I was thinking of some requests that have an old way of
> > > authorizing
> > > and a new way where we have retained the old way for backward
> > > compatibility. One example is Cluster:Create permission to create
> > > topics.
> > > We have replaced this with fine-grained topic create access using
> > > Topic:Create
> > > for topic patterns. But we still check if user has Cluster:Create
> > > first. If
> > > Denied, the deny is ignored and we check Topic:Create. We dont want
> > to
> > > log
> > > DENY for Cluster:Create at INFO level for this, since this is not a
> > > mandatory ACL for creating topics. We will get appropriate logs
> from
> > > the
> > > subsequent Topic:Create in this case.
> > > 3.3 They are not quite the same. FILTER implies that user actually
> > > requested access to and performed some operation on the filtered
> > > resources.
> > > LIST_AUTHORZED did not result in any actual access. User simply
> > wanted
> > > to
> > > know what they are allowed to access.
> > > 3.4 Request types are Produce, JoinGroup, OffsetCommit etc. So that
> > is
> > > different from authorization mode, operation etc.
> > >
> > >
> > > On Thu, Aug 8, 2019 at 11:36 PM Don Bosco Durai 
> > > wrote:
> > >
> > > > Hi Rajini
> > > >
> > > > Thanks for clarifying. This is very helpful...
> > > >
> > > > #1 - On the Ranger side, we should be able to handle multiple
> > > requests at
> > > > the same time. I was just not sure how much processing overhead
> > will
> > > be
> > > > there on the Broker side to split and then consolidate the
> results.
> > > If it
> > > > is negligible,  then this is the better way. It will make it
> future
> > > proof.
> > > > #2 -  I agree, having a single "start" call makes it cleaner. The
> > > > Authorization implementation will only have to do the
> > initialization
> > > only
> > > > once.
> > > > #3.1 - Thanks for the clarification. I thin

Re: [DISCUSS] KIP-482: The Kafka Protocol should Support Optional Fields

2019-08-13 Thread David Jacot
Hi Colin,

Thank you for the KIP! Things are well explained!. It is huge improvement
for the Kafka protocol. I have few comments on the proposal:

1. The interleaved tag/length header sounds like a great optimisation as it
would be shorter on average. The downside, as
you already pointed out, is that it makes the decoding and the specs more
complex. Personally, I would also favour using two
vaints in this particular case to keep things simple.

2. As discussed, I wonder if it would make sense to extend to KIP to also
support optional fields in the Record Header. I think
that it could be interesting to have such capability for common fields
across all the requests or responses (e.g. tracing id).

Regards,
David



On Tue, Aug 13, 2019 at 10:00 AM Jason Gustafson  wrote:

> > Right, I was planning on doing exactly that for all the auto-generated
> RPCs. For the manual RPCs, it would be a lot of work. It’s probably a
> better use of time to convert the manual ones to auto gen first (with the
> possible exception of Fetch/Produce, where the ROI may be higher for the
> manual work)
>
> Yeah, that makes sense. Maybe we can include the version bump for all RPCs
> in this KIP, but we can implement it lazily as the protocols are converted.
>
> -Jason
>
> On Mon, Aug 12, 2019 at 7:16 PM Colin McCabe  wrote:
>
> > On Mon, Aug 12, 2019, at 11:22, Jason Gustafson wrote:
> > > Hi Colin,
> > >
> > > Thanks for the KIP! This is a significant improvement. One of my
> personal
> > > interests in this proposal is solving the compatibility problems we
> have
> > > with the internal schemas used to define consumer offsets and
> transaction
> > > metadata. Currently we have to guard schema bumps with the inter-broker
> > > protocol format. Once the format is bumped, there is no way to
> downgrade.
> > > By fixing this, we can potentially begin using the new schemas before
> the
> > > IBP is bumped while still allowing downgrade.
> > >
> > > There are a surprising number of other situations we have encountered
> > this
> > > sort of problem. We have hacked around it in special cases by allowing
> > > nullable fields to the end of the schema, but this is not really an
> > > extensible approach. I'm looking forward to having a better option.
> >
> > Yeah, this problem keeps coming up.
> >
> > >
> > > With that said, I have a couple questions on the proposal:
> > >
> > > 1. For each request API, we need one version bump to begin support for
> > > "flexible versions." Until then, we won't have the option of using
> tagged
> > > fields even if the broker knows how to handle them. Does it make sense
> to
> > > go ahead and do a universal bump of each request API now so that we'll
> > have
> > > this option going forward?
> >
> > Right, I was planning on doing exactly that for all the auto-generated
> > RPCs. For the manual RPCs, it would be a lot of work. It’s probably a
> > better use of time to convert the manual ones to auto gen first (with the
> > possible exception of Fetch/Produce, where the ROI may be higher for the
> > manual work)
> >
> > > 2. The alternating length/tag header encoding lets us save a byte in
> the
> > > common case. The downside is that it's a bit more complex to specify.
> It
> > > also has some extra cost if the length exceeds the tag substantially.
> > > Basically we'd have to pad the tag, right? I think I'm wondering if we
> > > should just bite the bullet and use two varints instead.
> >
> > That’s a fair point. It would be shorter on average, but worse for some
> > exceptional cases. Also, the decoding would be more complex, which might
> be
> > a good reason to go for just having two varints. Yeah, let’s simplify.
> >
> > Regards,
> > Colin
> >
> > >
> > > Thanks,
> > > Jason
> > >
> > >
> > > On Fri, Aug 9, 2019 at 4:31 PM Colin McCabe 
> wrote:
> > >
> > > > Hi all,
> > > >
> > > > I've made some updates to this KIP. Specifically, I wanted to avoid
> > > > including escape bytes in the serialization format, since it was too
> > > > complex. Also, I think this is a good opportunity to slim down our
> > > > variable length fields.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Thu, Jul 11, 2019, at 20:52, Colin McCabe wrote:
> > > > > On Tue, Jul 9, 2019, at 15:29, Jose Armando Garcia Sancio wrote:
> > > > > > Thanks Colin for the KIP. For my own edification why are we doing
> > this
> > > > > > "Optional fields can have any type, except for an array of
> > > > structures."?
> > > > > > Why can't we have an array of structures?
> > > > >
> > > > > Optional fields are serialized starting with their total length.
> This
> > > > > is straightforward to calculate for primitive fields like INT32,
> (or
> > > > > even an array of INT32), but more difficult to calculate for an
> array
> > > > > of structures. Basically, we'd have to do a two-pass serialization
> > > > > where we first calculate the lengths of everything, and then write
> it
> > > > > out.
> > > > >
> > > > > The nice thing abo

Re: [VOTE] KIP-503: deleted topics metric

2019-08-14 Thread David Jacot
+1 (non-binding)

Thanks for the KIP! Simple yet very useful.

Best,
David

On Wed, Aug 14, 2019 at 9:24 AM Robert Barrett 
wrote:

> +1 (non-binding)
>
> This will be good to have, thanks David!
>
> Bob
>
> On Wed, Aug 14, 2019 at 6:08 AM Mickael Maison 
> wrote:
>
> > +1 non binding
> > Thank you!
> >
> > On Tue, Aug 13, 2019 at 9:07 PM Stanislav Kozlovski
> >  wrote:
> > >
> > > +1 (non-binding)
> > >
> > > Thanks for the simple but very useful KIP!
> > > Best,
> > > Stanislav
> > >
> > > On Tue, Aug 13, 2019 at 8:32 PM Harsha Chintalapani 
> > wrote:
> > >
> > > > +1 (binding)
> > > >
> > > > Thanks,
> > > > Harsha
> > > >
> > > >
> > > > On Tue, Aug 13, 2019 at 12:08 PM, David Arthur <
> davidart...@apache.org
> > >
> > > > wrote:
> > > >
> > > > > Hello all,
> > > > >
> > > > > I'd like to start the vote on KIP-503
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/
> > > > > KIP-503%3A+Add+metric+for+number+of+topics+marked+for+deletion
> > > > >
> > > > > Thanks!
> > > > > David
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best,
> > > Stanislav
> >
>


Please add me to the contributor list

2015-07-16 Thread David Jacot
Hi Kafka team,

First of all, thank you for the awesome work! Kafka is really an awesome
piece of technology. I have been using it since you open sourced it and
would like to contribute to it.

Could you please add me to the contributor list?

Thanks!
David Jacot


Re: [DISCUSS] KIP-482: The Kafka Protocol should Support Optional Fields

2019-08-19 Thread David Jacot
Hi Colin,

Thank you for the updated KIP.

wrt. Request and Response Headers v1, where do you plan to put the optional
fields? You mention that all the Requests and Responses will start with a
tagged field buffer. Is it for that purpose?

Best,
David

On Mon, Aug 19, 2019 at 7:27 AM Satish Duggana 
wrote:

> Please read struct type as a complex record type in my earlier mail.
> The complex type seems to be defined as Schema[1] in the protocol
> types.
>
> 1.
> https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/protocol/types/Schema.java#L27
>
>
> On Mon, Aug 19, 2019 at 9:46 AM Satish Duggana 
> wrote:
> >
> > Sorry! Colin, I may not have been clear in my earlier query about
> > optional field type restriction. It is mentioned in one of your
> > replies "optional fields are serialized starting with their total
> > length". This brings the question of whether optional fields support
> > struct types (with or without array values). It seems struct types are
> > currently not serialized with total length. I may be missing something
> > here.
> >
> > Thanks,
> > Satish.
> >
> >
> > On Wed, Aug 14, 2019 at 8:03 AM Satish Duggana 
> wrote:
> > >
> > > Hi Colin,
> > > Thanks for the KIP. Optional fields and var length encoding support is
> a great
> > > improvement for the protocol.
> > >
> > > >>Optional fields can have any type, except that they cannot be arrays.
> > > Note that the restriction against having tagged arrays is just to
> simplify
> > > serialization.  We can relax this restriction in the future without
> changing
> > > the protocol on the wire.
> > >
> > > Can an Optional field have a struct type which internally contains an
> array
> > > field at any level?
> > >
> > > Thanks,
> > > Satish.
> > >
> > >
> > >
> > > On Tue, Aug 13, 2019 at 11:49 PM David Jacot 
> wrote:
> > > >
> > > > Hi Colin,
> > > >
> > > > Thank you for the KIP! Things are well explained!. It is huge
> improvement
> > > > for the Kafka protocol. I have few comments on the proposal:
> > > >
> > > > 1. The interleaved tag/length header sounds like a great
> optimisation as it
> > > > would be shorter on average. The downside, as
> > > > you already pointed out, is that it makes the decoding and the specs
> more
> > > > complex. Personally, I would also favour using two
> > > > vaints in this particular case to keep things simple.
> > > >
> > > > 2. As discussed, I wonder if it would make sense to extend to KIP to
> also
> > > > support optional fields in the Record Header. I think
> > > > that it could be interesting to have such capability for common
> fields
> > > > across all the requests or responses (e.g. tracing id).
> > > >
> > > > Regards,
> > > > David
> > > >
> > > >
> > > >
> > > > On Tue, Aug 13, 2019 at 10:00 AM Jason Gustafson 
> wrote:
> > > >
> > > > > > Right, I was planning on doing exactly that for all the
> auto-generated
> > > > > RPCs. For the manual RPCs, it would be a lot of work. It’s
> probably a
> > > > > better use of time to convert the manual ones to auto gen first
> (with the
> > > > > possible exception of Fetch/Produce, where the ROI may be higher
> for the
> > > > > manual work)
> > > > >
> > > > > Yeah, that makes sense. Maybe we can include the version bump for
> all RPCs
> > > > > in this KIP, but we can implement it lazily as the protocols are
> converted.
> > > > >
> > > > > -Jason
> > > > >
> > > > > On Mon, Aug 12, 2019 at 7:16 PM Colin McCabe 
> wrote:
> > > > >
> > > > > > On Mon, Aug 12, 2019, at 11:22, Jason Gustafson wrote:
> > > > > > > Hi Colin,
> > > > > > >
> > > > > > > Thanks for the KIP! This is a significant improvement. One of
> my
> > > > > personal
> > > > > > > interests in this proposal is solving the compatibility
> problems we
> > > > > have
> > > > > > > with the internal schemas used to define consumer offsets and
> > > > > transaction
> > > > > > > metadata. Currently we have to guard schema bumps with th

Re: [VOTE] KIP-504 - Add new Java Authorizer Interface

2019-08-19 Thread David Jacot
+1 (non-binding)

Thanks for the KIP, Rajini.

Best,
David

On Sun, Aug 18, 2019 at 9:42 PM Mickael Maison 
wrote:

> +1 (non binding)
> Thanks for the KIP!
>
> On Sun, Aug 18, 2019 at 3:05 PM Ron Dagostino  wrote:
> >
> > +1 (non-binding)
> >
> > Thanks, Rajini.
> >
> > Ron
> >
> > On Sat, Aug 17, 2019 at 10:16 AM Harsha Chintalapani 
> > wrote:
> >
> > > +1 (binding).
> > >
> > > Thanks,
> > > Harsha
> > >
> > >
> > > On Sat, Aug 17, 2019 at 2:53 AM, Manikumar 
> > > wrote:
> > >
> > > > Hi,
> > > >
> > > > +1 (binding).
> > > >
> > > > Thanks for the KIP. LGTM.
> > > >
> > > > Regards,
> > > > Manikumar
> > > >
> > > > On Sat, Aug 17, 2019 at 4:41 AM Colin McCabe 
> wrote:
> > > >
> > > > +1 (binding)
> > > >
> > > > Thanks, Rajini!
> > > >
> > > > best,
> > > > Colin
> > > >
> > > > On Fri, Aug 16, 2019, at 09:52, Rajini Sivaram wrote:
> > > >
> > > > Hi all,
> > > >
> > > > I would like to start the vote for KIP-504:
> > > >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/
> > > > KIP-504+-+Add+new+Java+Authorizer+Interface
> > > >
> > > > This KIP replaces the Scala Authorizer API with a new Java API
> similar to
> > > > other pluggable APIs in the broker and addresses known limitations
> in the
> > > > existing API.
> > > >
> > > > Thanks for all the feedback!
> > > >
> > > > Regards,
> > > >
> > > > Rajini
> > > >
> > > >
> > >
>


Permission to create KIP

2019-08-20 Thread David Jacot
Hey,

I would like to create a KIP but I don't have the permission yet. Could
someone grant me the relevant permission to the wiki?

Thanks,
David


Re: Permission to create KIP

2019-08-20 Thread David Jacot
Hi Bill,

Sure. It is dajac.

Thanks,
David

On Tue, Aug 20, 2019 at 3:55 PM Bill Bejeck  wrote:

> Hi David,
>
> Sure thing, can you share your wiki username?
>
> Thanks,
> Bill
>
> On Tue, Aug 20, 2019 at 9:53 AM David Jacot  wrote:
>
> > Hey,
> >
> > I would like to create a KIP but I don't have the permission yet. Could
> > someone grant me the relevant permission to the wiki?
> >
> > Thanks,
> > David
> >
>


Re: Permission to create KIP

2019-08-20 Thread David Jacot
Thank you, Bill!

Le mar. 20 août 2019 à 16:45, Bill Bejeck  a écrit :

> Hi David,
>
> You're all set now.
>
> -Bill
>
> On Tue, Aug 20, 2019 at 10:04 AM David Jacot  wrote:
>
> > Hi Bill,
> >
> > Sure. It is dajac.
> >
> > Thanks,
> > David
> >
> > On Tue, Aug 20, 2019 at 3:55 PM Bill Bejeck  wrote:
> >
> > > Hi David,
> > >
> > > Sure thing, can you share your wiki username?
> > >
> > > Thanks,
> > > Bill
> > >
> > > On Tue, Aug 20, 2019 at 9:53 AM David Jacot 
> wrote:
> > >
> > > > Hey,
> > > >
> > > > I would like to create a KIP but I don't have the permission yet.
> Could
> > > > someone grant me the relevant permission to the wiki?
> > > >
> > > > Thanks,
> > > > David
> > > >
> > >
> >
>


Re: [VOTE] KIP-352: Distinguish URPs caused by reassignment

2019-08-21 Thread David Jacot
+1 (non-binding)

Thanks for the KIP!

Best,
David

On Tue, Aug 20, 2019 at 7:55 PM Jason Gustafson  wrote:

> Hi All,
>
> I'd like to start a vote on KIP-352, which is a follow-up to KIP-455 to fix
> a long-known shortcoming of URP reporting and to improve reassignment
> monitoring:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-352%3A+Distinguish+URPs+caused+by+reassignment
> .
>
> Note that I have added one new metric following the discussion. It seemed
> useful to have a lag metric for reassigning partitions.
>
> Thanks,
> Jason
>


Re: ACL for group creation?

2019-08-21 Thread David Jacot
Hello,

It would be better to ask such question on the user mailing list.

The reason is that the group is created automatically when a consumer
joins it. It is not created explicitly so it can be restricted.

In your case, you could setup a ACL to authorize the application to only
use the group you have defined. It would prevent the application from
creating new groups. (READ Acl on Group resource with a specific name).

Best,
David

On Mon, Aug 19, 2019 at 9:01 PM Adam Bellemare 
wrote:

> Hi All
>
> I am looking through the Confluent docs and core Kafka docs and don't see
> an ACL for group creation:
> https://docs.confluent.io/current/kafka/authorization.html#acl-format
> and
> https://kafka.apache.org/documentation/#security_authz
>
> My scenario is simple: We use the consumer group as the means of
> identifying a single application, including tooling for managing
> application resets, offset management, lag monitoring, etc. We often have
> situations where someone resets their consumer group by appending an
> incremented integer ("cg" to "cg1"), but it throws the rest of the
> monitoring and management tooling out of whack.
>
> Is there a reason why we do not have ACL-based CREATE restrictions to a
> particular consumer group? I am willing to do the work to implement this
> and test it out, but I wanted to validate that there isn't a reason I am
> missing.
>
> Thanks
> Adam
>


[DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-08-21 Thread David Jacot
Hi all,

I would like to start a discussion for KIP-511:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers

Let me know what you think.

Best,
David


Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-08-22 Thread David Jacot
Hi Satish,

Thank you for your feedback!

Please find my answers below.

>> Did you consider taking version property by loading
“kafka/kafka-version.properties” as a resource while java client is
initialized?  “kafka/kafka-version.properties” is shipped with
kafka-clients jar.

I wasn't aware of the property file. It is exactly what I need. Thanks for
pointing that out!

>> I assume this metric value will be the total no of clients connected
to a broker irrespective of whether name and version follow the
expected pattern ([-.\w]+) or not.

That is correct.

>> It seems client name and client version are treated as tags for
`ConnectedClients` metric. If so, you may implement this metric
similar to `BrokerTopicMetrics` with topic tag as mentioned here[1].
When is the metric removed for a specific client-name and
client-version?

That is correct. Client name and version are treated as tags like in
BrokerTopicMetrics. My plan is to remove the metric when it goes
back to zero - when all clients with a given name & version are
disconnected.

Best,
David

On Wed, Aug 21, 2019 at 6:52 PM Satish Duggana 
wrote:

> Hi David,
> Thanks for the KIP. I have a couple of questions.
>
> >> For the Java client, the idea is to define two constants in the code to
> store its name and its version. If possible, the version will be set
> automatically based on metadata coming from gradle (or the repo itself) to
> avoid having to do manual changes.
>
> Did you consider taking version property by loading
> “kafka/kafka-version.properties” as a resource while java client is
> initialized?  “kafka/kafka-version.properties” is shipped with
> kafka-clients jar.
>
> >> kafka.server:type=ClientMetrics,name=ConnectedClients
> I assume this metric value will be the total no of clients connected
> to a broker irrespective of whether name and version follow the
> expected pattern ([-.\w]+) or not.
>
> >>
> kafka.server:type=ClientMetrics,name=ConnectedClients,clientname=([-.\w]+),clientversion=([-.\w]+)
> It seems client name and client version are treated as tags for
> `ConnectedClients` metric. If so, you may implement this metric
> similar to `BrokerTopicMetrics` with topic tag as mentioned here[1].
> When is the metric removed for a specific client-name and
> client-version?
>
> 1.
> https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/KafkaRequestHandler.scala#L231
>
> Thanks,
> Satish.
>
>
>
>
> On Wed, Aug 21, 2019 at 5:33 PM David Jacot  wrote:
> >
> > Hi all,
> >
> > I would like to start a discussion for KIP-511:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers
> >
> > Let me know what you think.
> >
> > Best,
> > David
>


Re: ACL for group creation?

2019-08-22 Thread David Jacot
Hi Adam,

Assuming you have a cluster where access to any groups is denied by
default, you can allow
a principal to only have access to one group.

Something like the following:
kafka-acls.sh --add --allow-principal User:CN=my-principal --operation Read
--topic 'my-topic' --group 'my-group'

With this, *my-principal* will only be able to join the consumer group
*my-group*. If the principal
uses another group, such as *my-group2*, it will get:
org.apache.kafka.common.errors.GroupAuthorizationException: Not authorized
to access group: my-group2

I hope it helps.

Best,
David

On Thu, Aug 22, 2019 at 4:36 PM Adam Bellemare 
wrote:

> I see the consumer group much like the username in relational database
> access credentials. We routinely use the consumer group name as the means
> of identifying the consumer for operational things, such as alerting based
> on consumer group lag, autoscaling based on lag, and tooling around
> manipulating the consumer group offset for a particular service. The
> consumer group allows us to know, operationally, which offsets we are
> observing or manipulating, and ideally we would like to limit the set of
> consumer groups possible.
>
> In practice, I regularly see people simply append an incrementing integer
> to the end of their consumer group (cg, cg1, cg2, cg1234) when they want to
> reset their application, INSTEAD of following offset reset or kafka-streams
> application reset procedures. Sure, it would be nice to get everyone to
> follow recommended procedures, but people do what they CAN do, not what
> they're supposed to do. This has significant impact on the brokers and
> surrounding tooling (orphaned internal topics with indefinite retention,
> falsely-firing lag monitoring, broken auto-scaling), and instead of us
> building out layers of supportive tooling to isolate ourselves from it, why
> not simply set up ACLs to enforce which consumer groups an application can
> and cannot use?
>
> Does this need a KIP? Or is a bug report simply enough? I am unsure how
> compatibility would work, so I am leaning towards a KIP...
>
> Thanks
> Adam
>
> On Wed, Aug 21, 2019 at 5:59 PM Colin McCabe  wrote:
>
> > I think it's worth considering  separating out the permissions needed to
> > create a consumer group from the permissions needed to join one.  We
> > distinguish these permissions for topics, and people generally find it
> > useful.  We could start checking CREATE on GROUP, perhaps?  It might be
> > hard to do in a compatible way.
> >
> > cheers,
> > Colin
> >
> >
> > On Wed, Aug 21, 2019, at 12:05, Adam Bellemare wrote:
> > > +users mailing list
> > >
> > > David,
> > >
> > > I don't think I really understand your email. Are you saying that this
> > can
> > > already be achieved only using the READ ACL?
> > >
> > > Thanks
> > > Adam
> > >
> > >
> > >
> > > On Wed, Aug 21, 2019 at 3:58 AM David Jacot 
> wrote:
> > >
> > > > Hello,
> > > >
> > > > It would be better to ask such question on the user mailing list.
> > > >
> > > > The reason is that the group is created automatically when a consumer
> > > > joins it. It is not created explicitly so it can be restricted.
> > > >
> > > > In your case, you could setup a ACL to authorize the application to
> > only
> > > > use the group you have defined. It would prevent the application from
> > > > creating new groups. (READ Acl on Group resource with a specific
> name).
> > > >
> > > > Best,
> > > > David
> > > >
> > > > On Mon, Aug 19, 2019 at 9:01 PM Adam Bellemare <
> > adam.bellem...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi All
> > > > >
> > > > > I am looking through the Confluent docs and core Kafka docs and
> > don't see
> > > > > an ACL for group creation:
> > > > >
> > https://docs.confluent.io/current/kafka/authorization.html#acl-format
> > > > > and
> > > > > https://kafka.apache.org/documentation/#security_authz
> > > > >
> > > > > My scenario is simple: We use the consumer group as the means of
> > > > > identifying a single application, including tooling for managing
> > > > > application resets, offset management, lag monitoring, etc. We
> often
> > have
> > > > > situations where someone resets their consumer group by appending
> an
> > > > > incremented integer ("cg" to "cg1"), but it throws the rest of the
> > > > > monitoring and management tooling out of whack.
> > > > >
> > > > > Is there a reason why we do not have ACL-based CREATE restrictions
> > to a
> > > > > particular consumer group? I am willing to do the work to implement
> > this
> > > > > and test it out, but I wanted to validate that there isn't a reason
> > I am
> > > > > missing.
> > > > >
> > > > > Thanks
> > > > > Adam
> > > > >
> > > >
> > >
> >
>


Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-08-30 Thread David Jacot
gt; and providing more meaningful logs to the client user in case a feature
> > (based on the actual api versions) is not available.
>
> I can think of several cases where people tried to set up client-side
> hacks based on the broker version, and were only stopped by the fact that
> we don't expose this information.  I agree with Jay that we should think
> very carefully before exposing it.  In any case, this seems out of scope...
>
> best,
> Colin
>
> >
> > /Magnus
> >
> >
> > Den tors 22 aug. 2019 kl 10:09 skrev David Jacot :
> >
> > > Hi Satish,
> > >
> > > Thank you for your feedback!
> > >
> > > Please find my answers below.
> > >
> > > >> Did you consider taking version property by loading
> > > “kafka/kafka-version.properties” as a resource while java client is
> > > initialized?  “kafka/kafka-version.properties” is shipped with
> > > kafka-clients jar.
> > >
> > > I wasn't aware of the property file. It is exactly what I need. Thanks
> for
> > > pointing that out!
> > >
> > > >> I assume this metric value will be the total no of clients connected
> > > to a broker irrespective of whether name and version follow the
> > > expected pattern ([-.\w]+) or not.
> > >
> > > That is correct.
> > >
> > > >> It seems client name and client version are treated as tags for
> > > `ConnectedClients` metric. If so, you may implement this metric
> > > similar to `BrokerTopicMetrics` with topic tag as mentioned here[1].
> > > When is the metric removed for a specific client-name and
> > > client-version?
> > >
> > > That is correct. Client name and version are treated as tags like in
> > > BrokerTopicMetrics. My plan is to remove the metric when it goes
> > > back to zero - when all clients with a given name & version are
> > > disconnected.
> > >
> > > Best,
> > > David
> > >
> > > On Wed, Aug 21, 2019 at 6:52 PM Satish Duggana <
> satish.dugg...@gmail.com>
> > > wrote:
> > >
> > > > Hi David,
> > > > Thanks for the KIP. I have a couple of questions.
> > > >
> > > > >> For the Java client, the idea is to define two constants in the
> code
> > > to
> > > > store its name and its version. If possible, the version will be set
> > > > automatically based on metadata coming from gradle (or the repo
> itself)
> > > to
> > > > avoid having to do manual changes.
> > > >
> > > > Did you consider taking version property by loading
> > > > “kafka/kafka-version.properties” as a resource while java client is
> > > > initialized?  “kafka/kafka-version.properties” is shipped with
> > > > kafka-clients jar.
> > > >
> > > > >> kafka.server:type=ClientMetrics,name=ConnectedClients
> > > > I assume this metric value will be the total no of clients connected
> > > > to a broker irrespective of whether name and version follow the
> > > > expected pattern ([-.\w]+) or not.
> > > >
> > > > >>
> > > >
> > >
> kafka.server:type=ClientMetrics,name=ConnectedClients,clientname=([-.\w]+),clientversion=([-.\w]+)
> > > > It seems client name and client version are treated as tags for
> > > > `ConnectedClients` metric. If so, you may implement this metric
> > > > similar to `BrokerTopicMetrics` with topic tag as mentioned here[1].
> > > > When is the metric removed for a specific client-name and
> > > > client-version?
> > > >
> > > > 1.
> > > >
> > >
> https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/KafkaRequestHandler.scala#L231
> > > >
> > > > Thanks,
> > > > Satish.
> > > >
> > > >
> > > >
> > > >
> > > > On Wed, Aug 21, 2019 at 5:33 PM David Jacot 
> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > I would like to start a discussion for KIP-511:
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers
> > > > >
> > > > > Let me know what you think.
> > > > >
> > > > > Best,
> > > > > David
> > > >
> > >
> >
>


Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-03 Thread David Jacot
Hi all,

I have updated the KIP to address the various comments. I have also added a
section about the handling of the ApiVersionsRequest/Response in the broker.

Please, let me know what you think. I would like to make it for the next
release if possible.

Best,
David

On Fri, Aug 30, 2019 at 10:31 AM David Jacot  wrote:

> Hi Magnus,
>
> Thank you for your feedback. Please, find my comments below.
>
> 1. I thought that the clientId was meant for this purpose (providing
> information about the application). Is there a gap I don't see?
>
> 2. I have put two fields to avoid requiring deep validation and parsing on
> the broker. I believe that it will be easier to use the metadata downstream
> like this.
>
> 3. Good point. I think that the broker should have some sort of validation
> and fail the ApiVersionRequest and/or the Connection if the validation
> fails. To avoid backward compatibility issues, I think we should come up
> with a minimal validation and ensure it won't become more restrictive in
> the future.
>
> 4. I don't have strong opinion regarding this one but as the focus of the
> KIP is to gather the client's information, I suggest to discuss/address
> this later on.
>
> Best,
> David
>
> On Thu, Aug 29, 2019 at 6:56 PM Colin McCabe  wrote:
>
>> On Fri, Aug 23, 2019, at 00:07, Magnus Edenhill wrote:
>> > Great proposal, this feature is well overdue!
>> >
>> > 1)
>> > From an operator's perspective I don't think the kafka client
>> > implementation name and version are sufficient,
>> > I also believe the application name and version are of interest.
>> > You could have all applications in your cluster run the same kafka
>> client
>> > and version, but only one type or
>> > version of an application misbehave and needing to be tracked down.
>>
>> Hi Magnus,
>>
>> I think it might be better to leave this out of scope for now, and think
>> about it in the context of more generalized request tracing.  This is a
>> very deep rabbit hole, and I could easily see it delaying this KIP for a
>> long time.  For example, if you have multiple Spark jobs producing to
>> Kafka, just knowing that a client is being used by Spark may not be that
>> helpful.  So maybe you want a third set of fields to describe the spark
>> application ID and version, etc?  And then maybe that, itself, was created
>> by some framework... etc. Probably better to defer this discussion for now
>> and see how version tracking works out.
>>
>> >
>> > While the application and client name and version could be combined in
>> the
>> > ClientName/ClientVersion fields by
>> > the user (e.g. like User-Agent), it would not be in a generalized format
>> > and hard for generic monitoring tools to parse correctly.
>> >
>> > So I'd suggest keeping ClientName and ClientVersion as the client
>> > implementation name ("java" or "org.apache.kafka...") and version,
>> > which can't be changed by the user/app developer, and providing two
>> > optional fields for the application counterpart:
>> > ApplicationName and ApplicationVersion, which are backed by
>> corresponding
>> > configuration properties (application.name, application.version).
>> >
>> > 2)
>> > Do ..Name and ..Version need to be two separate fields, seeing how the
>> two
>> > fields are ambigious when separated?
>> > If we're looking to identify unique versions, combining the two fields
>> > would be sufficient (e.g., "java-2.3.1", "librdkafka/1.2.0",
>> "sarama@1.2.3")
>> > and perhaps easier to work with.
>> > The actual format or content of the name-version string is irrelevant as
>> > long as it identifies a unique name+version.
>> >
>>
>> Hmm.  Wouldn't the same arguments you made above about a combined
>> named+version field being "hard for generic monitoring tools to parse
>> correctly" apply here?  In any case, there seems to be no reason not to
>> just have two fields.  It avoids string parsing.
>>
>> >
>> > 3)
>> > As for allowed characters, will the broker fail the ApiVersionResponse
>> if
>> > any of these fields contain invalid characters,
>> > or will the broker sanitize the strings?
>> > For future backwards compatibility (when the broker constraints change
>> but
>> > clients are not updated) I suggest the latter.
>> >
>>
>> I would argue that we should be strict about the c

Re: [VOTE] KIP-482: The Kafka Protocol should Support Optional Tagged Fields

2019-09-03 Thread David Jacot
+1 (non-binding)

Thank for the KIP. Great addition to the Kafka protocol!

Best,
David

Le mar. 3 sept. 2019 à 19:17, Colin McCabe  a écrit :

> Hi all,
>
> I'd like to start the vote for KIP-482: The Kafka Protocol should Support
> Optional Tagged Fields.
>
> KIP:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-482%3A+The+Kafka+Protocol+should+Support+Optional+Tagged+Fields
>
> Discussion thread here:
> https://lists.apache.org/thread.html/cdc801ae886491b73ef7efecac7ef81b24382f8b6b025899ee343f7a@%3Cdev.kafka.apache.org%3E
>
> best,
> Colin
>


Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-03 Thread David Jacot
Hi Colin,

Thanks for your input. Please, find my comments below:

>> Currently, we don't parse the contents of ApiVersionsRequest at all,
since it's an empty message.  KIP-511 proposes adding some fields here,
which will clearly change that situation.  In the future, any changes to
ApiVersionsRequest will have to only add stuff at the end, rather than
changing fields that already exist.  This will significantly limit our
ability to add new things over time.
>> Therefore, I think we should make sure that KIP-511 is implemented after
KIP-482, so that the version we freeze into place can be one that includes
the ability to add tagged fields, and includes the more efficient string
and array serialization specified in KIP-482.  It's probably worth spelling
that out here.

I agree with you. It makes sense to bump the version once for the two
cases.

>> On another topic, when the client sends an unsupported version N of
ApiVersionsRequest, and the broker only supports versions up to M, the
broker currently falls back to sending a version 0 response.  Why can't the
broker fall back to version M instead?  Previously, we only had versions 1
and 0 of ApiVersionsRequest, so the two behaviors were identical.  But
going forward, once we have version 2 and later of ApiVersoinsRequest, it
doesn't make sense to fall back all the way to 0 if the broker supports 1
(for example).
>> If you agree, it would be good to spell this out in the KIP, so that if
we want to add more things to the response, we can, without losing them
each time the client's version of ApiVersionsRequest exceeds the broker's.

I fully agree with you and I have already outlined this in the proposal,
see "ApiVersions Request/Response Handling". The idea is to use the version
M in the broker so it can leverage the provided information it knows about.
The broker can then send back response with version M as well. On the
client side, it is a bit more tricky. As there is no version in the
response, the client will use the version used for the request to parse the
response. If fields have been added to the schema of the response version
N, it won't work to parse M. As the client doesn't know the version used,
we have two options: 1) use version 0 which is a prefix of all others but
it means losing information; or 2) try versions in descending order from N
to 0. I guess that N-1 would the one in most of the cases.

What is your opinion regarding the client side?

Best,
David

On Wed, Sep 4, 2019 at 12:13 AM Colin McCabe  wrote:

> Hi David,
>
> Thanks again for the KIP.
>
> Currently, we don't parse the contents of ApiVersionsRequest at all, since
> it's an empty message.  KIP-511 proposes adding some fields here, which
> will clearly change that situation.  In the future, any changes to
> ApiVersionsRequest will have to only add stuff at the end, rather than
> changing fields that already exist.  This will significantly limit our
> ability to add new things over time.
>
> Therefore, I think we should make sure that KIP-511 is implemented after
> KIP-482, so that the version we freeze into place can be one that includes
> the ability to add tagged fields, and includes the more efficient string
> and array serialization specified in KIP-482.  It's probably worth spelling
> that out here.
>
> On another topic, when the client sends an unsupported version N of
> ApiVersionsRequest, and the broker only supports versions up to M, the
> broker currently falls back to sending a version 0 response.  Why can't the
> broker fall back to version M instead?  Previously, we only had versions 1
> and 0 of ApiVersionsRequest, so the two behaviors were identical.  But
> going forward, once we have version 2 and later of ApiVersoinsRequest, it
> doesn't make sense to fall back all the way to 0 if the broker supports 1
> (for example).
>
> If you agree, it would be good to spell this out in the KIP, so that if we
> want to add more things to the response, we can, without losing them each
> time the client's version of ApiVersionsRequest exceeds the broker's.
>
> best,
> Colin
>
>
> On Tue, Sep 3, 2019, at 01:26, David Jacot wrote:
> > Hi all,
> >
> > I have updated the KIP to address the various comments. I have also
> added a
> > section about the handling of the ApiVersionsRequest/Response in the
> broker.
> >
> > Please, let me know what you think. I would like to make it for the next
> > release if possible.
> >
> > Best,
> > David
> >
> > On Fri, Aug 30, 2019 at 10:31 AM David Jacot 
> wrote:
> >
> > > Hi Magnus,
> > >
> > > Thank you for your feedback. Please, find my comments below.
> > >
> > > 1. I thought that the client

Re: [VOTE] KIP-496: Administrative API to delete consumer offsets

2019-09-04 Thread David Jacot
Hi all,

While implementing the KIP, I have realized that a new error code and
exception is required to notify the caller that offsets of a topic can not
be deleted because the group is actively subscribed to the topic.

I would like to know if there are any concerns with these changes before
updating the KIP.

*Proposed API:*
GROUP_SUBSCRIBED_TO_TOPIC(86, "The consumer group is actively subscribed to
the topic", GroupSubscribedToTopicException::new);

public class GroupSubscribedToTopicException extends ApiException {
public GroupSubscribedToTopicException(String message) {
super(message);
}
}

Best,
David

On Fri, Aug 16, 2019 at 10:58 AM Mickael Maison 
wrote:

> +1 (non binding)
> Thanks!
>
> On Thu, Aug 15, 2019 at 11:53 PM Colin McCabe  wrote:
> >
> > On Thu, Aug 15, 2019, at 11:47, Jason Gustafson wrote:
> > > Hey Colin, I think deleting all offsets is equivalent to deleting the
> > > group, which can be done with the `deleteConsumerGroups` api. I debated
> > > whether there should be a way to delete partitions for all unsubscribed
> > > topics, but I decided to start with a simple API.
> >
> > That's a fair point-- deleting the group covers the main use-case for
> deleting all offsets.  So we might as well keep it simple for now.
> >
> > cheers,
> > Colin
> >
> > >
> > > I'm going to close this vote. The final result is +3 with myself,
> Guozhang,
> > > and Colin voting.
> > >
> > > -Jason
> > >
> > > On Tue, Aug 13, 2019 at 9:21 AM Colin McCabe 
> wrote:
> > >
> > > > Hi Jason,
> > > >
> > > > Thanks for the KIP.
> > > >
> > > > Is there ever a desire to delete all the offsets for a given group?
> > > > Should the protocol and tools support this?
> > > >
> > > > +1 (binding)
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Mon, Aug 12, 2019, at 10:57, Guozhang Wang wrote:
> > > > > +1 (binding).
> > > > >
> > > > > Thanks Jason!
> > > > >
> > > > > On Wed, Aug 7, 2019 at 11:18 AM Jason Gustafson <
> ja...@confluent.io>
> > > > wrote:
> > > > >
> > > > > > Hi All,
> > > > > >
> > > > > > I'd like to start a vote on KIP-496:
> > > > > >
> > > > > >
> > > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-496%3A+Administrative+API+to+delete+consumer+offsets
> > > > > > .
> > > > > > +1
> > > > > > from me of course.
> > > > > >
> > > > > > -Jason
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > >
>


Re: [DISCUSS] Apache Kafka 2.4.0 release

2019-09-06 Thread David Jacot
Hi Manikumar,

Could we add KIP-511 to the plan? I think it will make it.

Thanks,
David

On Tue, Aug 27, 2019 at 5:32 PM Manikumar  wrote:

> Hi all,
>
> I put together a draft release plan with Oct 2019 as the release month and
> a list of KIPs that have already been voted:
>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=125307901
>
> Here are the dates:
>
> 1) KIP Freeze:  Sep 25, 2019 (A KIP must be accepted by this date in order
> to be considered for this release)
>
> 2) Feature Freeze:  Oct 2, 2019 (Major features merged & working on
> stabilization, minor features have PR,
>  release branch cut; anything not in this state will be automatically moved
> to the next release in JIRA.
>
> 3) Code Freeze:  Oct 16, 2019
>
> 4) Release Date: Oct 30, 2019 (tentative)
>
> Please plan accordingly for the features you want push into Apache Kafka
> 2.4.0 release.
>
> Regards,
> Manikumar
>
> On Mon, Aug 12, 2019 at 9:08 PM Ismael Juma  wrote:
>
> > Thanks for volunteering Manikumar. +1
> >
> > Ismael
> >
> > On Mon, Aug 12, 2019 at 7:54 AM Manikumar 
> > wrote:
> >
> > > Hi all,
> > >
> > > I would like to volunteer to be the release manager for our next
> > time-based
> > > feature release (v2.4.0).
> > >
> > > If that sounds good, I'll post the release plan over the next few days.
> > >
> > > Thanks,
> > > Manikumar
> > >
> >
>


Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-09 Thread David Jacot
t, it should fail all
the way back
to version 0 of the header and the request. (This requires clientid to
remain as Nullable
String in the RequestHeader v1 in KIP-482.) It means that name and version
of the client
wouldn't be available when the broker fails back to version 0 which is
pointless.  Knowing
this, it is preferable to go with option B) and avoid touching the
ApiVersionsRequest ever.

Best,
David

On Mon, Sep 9, 2019 at 3:18 AM Gwen Shapira  wrote:

> Hey,
>
> Since modifying ApiVersionsRequest seems to be quite involved, do we
> want to re-examine the rejected option of adding another
> request/response pair? It will add another roundtrip, but Kafka
> already expects client connections to be long-lived, so the overhead
> is probably negligible.
>
> In addition, in the rejected alternatives it says: "It also would
> require to be done before the authentication (TLS AuthN aside) and
> thus requiring specific treatment, similarly to the
> ApiVersionsRequest." - which I don't quite understand. Why do we think
> this has to happen before authenticating?
>
> It sounds like addition another protocol will allow us to keep the
> special assumptions around ApiVersionsRequest and will remove the
> dependency on KIP-482. Which will make KIP-511 much simpler than the
> alternative we are currently discussing.
>
> Gwen
>
> On Fri, Sep 6, 2019 at 3:16 PM Colin McCabe  wrote:
> >
> > Hi David,
> >
> > Yeah, this is kind of difficult.
> >
> > From the high level, we either need forward compatibility (old brokers
> able to read new ApiVersionsRequests) or some kind of challenge/response
> system.  I think you're on the right track to be thinking about forward
> compatibility...  a challenge/response system would have a lot of overhead
> in cases where the client opens a lot of new connections.
> >
> > I agree that we can have the new versions "add stuff at the end" to
> maintain forward compatibility.  Tagged fields will also help here, by
> avoiding the need for version bumps.  We also have to think about the
> impact this will have on the request header.  It seems like we might need
> to freeze the version of the request header forever in order to support
> full forwards compatibility.  Otherwise, the old brokers will not know how
> long the new request headers are.  This is maybe OK if we can evolve the
> request header by adding tagged fields...
> >
> > Another option is to fall all the way back to version 0 when the broker
> doesn't understand the client's supplied version.  Version 0 had no content
> in the request at all, so there is no need for the broker to know exactly
> how long the supplied request header is.  (This assumes that the first 4
> fields of the request header will never change, which seems like a
> reasonable assumption...)
> >
> > best,
> > Colin
> >
> >
> > On Tue, Sep 3, 2019, at 23:54, David Jacot wrote:
> > > Hi Colin,
> > >
> > > Thanks for your input. Please, find my comments below:
> > >
> > > >> Currently, we don't parse the contents of ApiVersionsRequest at all,
> > > since it's an empty message.  KIP-511 proposes adding some fields here,
> > > which will clearly change that situation.  In the future, any changes
> to
> > > ApiVersionsRequest will have to only add stuff at the end, rather than
> > > changing fields that already exist.  This will significantly limit our
> > > ability to add new things over time.
> > > >> Therefore, I think we should make sure that KIP-511 is implemented
> after
> > > KIP-482, so that the version we freeze into place can be one that
> includes
> > > the ability to add tagged fields, and includes the more efficient
> string
> > > and array serialization specified in KIP-482.  It's probably worth
> spelling
> > > that out here.
> > >
> > > I agree with you. It makes sense to bump the version once for the two
> > > cases.
> > >
> > > >> On another topic, when the client sends an unsupported version N of
> > > ApiVersionsRequest, and the broker only supports versions up to M, the
> > > broker currently falls back to sending a version 0 response.  Why
> can't the
> > > broker fall back to version M instead?  Previously, we only had
> versions 1
> > > and 0 of ApiVersionsRequest, so the two behaviors were identical.  But
> > > going forward, once we have version 2 and later of ApiVersoinsRequest,
> it
> > > doesn't make sense to fall back all the way to 0 if the broker
> supports 1
> > > (for exampl

Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-09 Thread David Jacot
Hi Gwen,

The reasoning behind having the information before the Sasl authentication
was to have the information for troubleshooting purposes. For instance, when
there are errors in the handshake, it would be great to know if it comes
from
a specific buggy clients. I think we could live without though but was nice
to
have.

Yeah. I agree with you. It seems that evolution of the RequestHeader (RH)
and the
ApiVersionsRequest (AVR) is conjunction is more complicated than I
anticipated.
The evolution if the AVR alone works well but it would make the evolution
of the
RH hard in the future. Please, check my other email to see my thoughts.

Best,
David

On Mon, Sep 9, 2019 at 3:18 AM Gwen Shapira  wrote:

> Hey,
>
> Since modifying ApiVersionsRequest seems to be quite involved, do we
> want to re-examine the rejected option of adding another
> request/response pair? It will add another roundtrip, but Kafka
> already expects client connections to be long-lived, so the overhead
> is probably negligible.
>
> In addition, in the rejected alternatives it says: "It also would
> require to be done before the authentication (TLS AuthN aside) and
> thus requiring specific treatment, similarly to the
> ApiVersionsRequest." - which I don't quite understand. Why do we think
> this has to happen before authenticating?
>
> It sounds like addition another protocol will allow us to keep the
> special assumptions around ApiVersionsRequest and will remove the
> dependency on KIP-482. Which will make KIP-511 much simpler than the
> alternative we are currently discussing.
>
> Gwen
>
> On Fri, Sep 6, 2019 at 3:16 PM Colin McCabe  wrote:
> >
> > Hi David,
> >
> > Yeah, this is kind of difficult.
> >
> > From the high level, we either need forward compatibility (old brokers
> able to read new ApiVersionsRequests) or some kind of challenge/response
> system.  I think you're on the right track to be thinking about forward
> compatibility...  a challenge/response system would have a lot of overhead
> in cases where the client opens a lot of new connections.
> >
> > I agree that we can have the new versions "add stuff at the end" to
> maintain forward compatibility.  Tagged fields will also help here, by
> avoiding the need for version bumps.  We also have to think about the
> impact this will have on the request header.  It seems like we might need
> to freeze the version of the request header forever in order to support
> full forwards compatibility.  Otherwise, the old brokers will not know how
> long the new request headers are.  This is maybe OK if we can evolve the
> request header by adding tagged fields...
> >
> > Another option is to fall all the way back to version 0 when the broker
> doesn't understand the client's supplied version.  Version 0 had no content
> in the request at all, so there is no need for the broker to know exactly
> how long the supplied request header is.  (This assumes that the first 4
> fields of the request header will never change, which seems like a
> reasonable assumption...)
> >
> > best,
> > Colin
> >
> >
> > On Tue, Sep 3, 2019, at 23:54, David Jacot wrote:
> > > Hi Colin,
> > >
> > > Thanks for your input. Please, find my comments below:
> > >
> > > >> Currently, we don't parse the contents of ApiVersionsRequest at all,
> > > since it's an empty message.  KIP-511 proposes adding some fields here,
> > > which will clearly change that situation.  In the future, any changes
> to
> > > ApiVersionsRequest will have to only add stuff at the end, rather than
> > > changing fields that already exist.  This will significantly limit our
> > > ability to add new things over time.
> > > >> Therefore, I think we should make sure that KIP-511 is implemented
> after
> > > KIP-482, so that the version we freeze into place can be one that
> includes
> > > the ability to add tagged fields, and includes the more efficient
> string
> > > and array serialization specified in KIP-482.  It's probably worth
> spelling
> > > that out here.
> > >
> > > I agree with you. It makes sense to bump the version once for the two
> > > cases.
> > >
> > > >> On another topic, when the client sends an unsupported version N of
> > > ApiVersionsRequest, and the broker only supports versions up to M, the
> > > broker currently falls back to sending a version 0 response.  Why
> can't the
> > > broker fall back to version M instead?  Previously, we only had
> versions 1
> > > and 0 of ApiVersionsRequest, so the two behav

Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-11 Thread David Jacot
Hi all,

I have discussed with Magnus about the various options to get his view from
a librdkafka perspective
and he has suggested a good alternative.

It seems we could continue with the idea to use the
ApiVersionsRequest/Response but we a different
failing back strategy. When a broker get an unknown ApiVersionsRequest, it
could continue to fail back
to version 0 as today but instead of sending back the UNSUPPORTED_VERSION
error alone in the
response, it could also provide at minimum the supported version of the
ApiVersionsRequest. This way,
a recent client could leverage that information when the error is received
and send the correct version
to the broker instead of failing all the way back to version 0.

This way, we can evolve the ApiVersionsRequest while keeping forward
compatibility of the Request
Header. It doesn't add any extra round trip.

Knowing this, I think that putting the information in the
ApiVersionsRequest remains the best option.

What do you think?

Best,
David

On Tue, Sep 10, 2019 at 1:00 AM Colin McCabe  wrote:

> Hi all,
>
> I agree that freezing the request header is not very appealing.  We might
> want to add something there later.
>
> Having a separate request type is very clean, but as David mentioned, it
> does add an extra request/response cycle to the time required to get a
> connection into a usable state.
>
> One solution to consider is adding the clientVersion and clientType to the
> request header as optional (tagged) fields.  This would let us skip the
> extra round trip.  I don't think it's that much more messy than having a
> separate request type to set the client version and type.  In both cases,
> you have to handle connections that set the version later than others, or
> don't set the version at all (for compatibility).  So the version/type has
> to be mutable and added after the TCP connection itself is established.
>
> best,
> Colin
>
>
> On Mon, Sep 9, 2019, at 06:11, David Jacot wrote:
> > Hi Gwen,
> >
> > The reasoning behind having the information before the Sasl
> authentication
> > was to have the information for troubleshooting purposes. For instance,
> when
> > there are errors in the handshake, it would be great to know if it comes
> > from
> > a specific buggy clients. I think we could live without though but was
> nice
> > to
> > have.
> >
> > Yeah. I agree with you. It seems that evolution of the RequestHeader (RH)
> > and the
> > ApiVersionsRequest (AVR) is conjunction is more complicated than I
> > anticipated.
> > The evolution if the AVR alone works well but it would make the evolution
> > of the
> > RH hard in the future. Please, check my other email to see my thoughts.
> >
> > Best,
> > David
> >
> > On Mon, Sep 9, 2019 at 3:18 AM Gwen Shapira  wrote:
> >
> > > Hey,
> > >
> > > Since modifying ApiVersionsRequest seems to be quite involved, do we
> > > want to re-examine the rejected option of adding another
> > > request/response pair? It will add another roundtrip, but Kafka
> > > already expects client connections to be long-lived, so the overhead
> > > is probably negligible.
> > >
> > > In addition, in the rejected alternatives it says: "It also would
> > > require to be done before the authentication (TLS AuthN aside) and
> > > thus requiring specific treatment, similarly to the
> > > ApiVersionsRequest." - which I don't quite understand. Why do we think
> > > this has to happen before authenticating?
> > >
> > > It sounds like addition another protocol will allow us to keep the
> > > special assumptions around ApiVersionsRequest and will remove the
> > > dependency on KIP-482. Which will make KIP-511 much simpler than the
> > > alternative we are currently discussing.
> > >
> > > Gwen
> > >
> > > On Fri, Sep 6, 2019 at 3:16 PM Colin McCabe 
> wrote:
> > > >
> > > > Hi David,
> > > >
> > > > Yeah, this is kind of difficult.
> > > >
> > > > From the high level, we either need forward compatibility (old
> brokers
> > > able to read new ApiVersionsRequests) or some kind of
> challenge/response
> > > system.  I think you're on the right track to be thinking about forward
> > > compatibility...  a challenge/response system would have a lot of
> overhead
> > > in cases where the client opens a lot of new connections.
> > > >
> > > > I agree that we can have the new versions "add stuff at the end" to
> > > maintain forward compatibility. 

Re: Kafka SSH Tunnel Connection without editing hostfile

2019-09-12 Thread David Jacot
Hey,

I have done this before with this proxy:
https://github.com/grepplabs/kafka-proxy#connect-to-kafka-through-socks5-proxy-example

You can spin up a socks proxy when you ssh to your jumphost (-D argument if
not mistaken) and
configure the proxy as described in the readme.

It is good for dev and test purposes. I wouldn't do this in production.

Best,
David

On Thu, Sep 12, 2019 at 11:23 AM Akshay Das 
wrote:

> Hi Team,
>
> I'm trying to consume from a kafka cluster using java client, but the kafka
> server can only be accessed via jumphost/ssh tunnel. But even after
> creating ssh tunnel we are not able to read because once conusmer fetches
> metadata it uses original hosts to connect to broker. Is it possible to
> stop this behaviour?
>
> Thanks,
> Akshay Das
>


Re: [VOTE] KIP-500: Replace ZooKeeper with a Self-Managed Metadata Quorum

2019-09-12 Thread David Jacot
+1 (non-binding)

Le jeu. 12 sept. 2019 à 20:35, Gwen Shapira  a écrit :

> +1 (binding)
>
> On Mon, Sep 9, 2019 at 8:28 AM Colin McCabe  wrote:
> >
> > Hi all,
> >
> > I'd like to start the vote for KIP-500: Replace ZooKeeper with a
> Self-Managed Metadata Quorum.
> >
> > The DISCUSS thread from the mailing list is here:
> >
> https://lists.apache.org/thread.html/cce5313ebe72bde34bf0da3af5a1723db3ee871667b1fd8edf2ee7ab@%3Cdev.kafka.apache.org%3E
> >
> > The KIP is here:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-500%3A+Replace+ZooKeeper+with+a+Self-Managed+Metadata+Quorum
> >
> > regards,
> > Colin
>


Re: [VOTE] KIP-496: Administrative API to delete consumer offsets

2019-09-13 Thread David Jacot
Hi all,

I would like to do another modification to the proposal. In the proposal,
the OffsetDeleteResponse
doesn't have a top level error field so I would like to add one. Many
errors concern the whole
group (e.g. GROUP_ID_NOT_FOUND) so it would be great to have a way to
communicate them
back to the client without having to set such errors for all the requested
partitions. It makes the
error handling on the client easier and cleaner.

*Proposed API with the ErrorCode:*
{
  "apiKey": 47,
  "type": "response",
  "name": "OffsetDeleteResponse",
  "validVersions": "0",
  "fields": [
{ "name": "ErrorCode", "type": "int16", "versions": "0+",
  "about": "The top-level error code, or 0 if there was no error." },
{ "name": "ThrottleTimeMs",  "type": "int32",  "versions": "0+",
"ignorable": true,
  "about": "The duration in milliseconds for which the request was
throttled due to a quota violation, or zero if the request did not violate
any quota." },
{ "name": "Topics", "type": "[]OffsetDeleteResponseTopic", "versions":
"0+",
  "about": "The responses for each topic.", "fields": [
{ "name": "Name", "type": "string", "versions": "0+", "mapKey":
true,
  "about": "The topic name." },
{ "name": "Partitions", "type": "[]OffsetDeleteResponsePartition",
"versions": "0+",
  "about": "The responses for each partition in the topic.",
"fields": [
{ "name": "PartitionIndex", "type": "int32", "versions": "0+",
"mapKey": true,
  "about": "The partition index." },
{ "name": "ErrorCode", "type": "int16", "versions": "0+",
  "about": "The error code, or 0 if there was no error." }
  ]
}
  ]
}
  ]
}

I would like to know if there are any concerns or objections regarding this
change before updating the KIP.

Best,
David

On Wed, Sep 4, 2019 at 9:24 AM David Jacot  wrote:

> Hi all,
>
> While implementing the KIP, I have realized that a new error code and
> exception is required to notify the caller that offsets of a topic can not
> be deleted because the group is actively subscribed to the topic.
>
> I would like to know if there are any concerns with these changes before
> updating the KIP.
>
> *Proposed API:*
> GROUP_SUBSCRIBED_TO_TOPIC(86, "The consumer group is actively subscribed
> to the topic", GroupSubscribedToTopicException::new);
>
> public class GroupSubscribedToTopicException extends ApiException {
> public GroupSubscribedToTopicException(String message) {
> super(message);
> }
> }
>
> Best,
> David
>
> On Fri, Aug 16, 2019 at 10:58 AM Mickael Maison 
> wrote:
>
>> +1 (non binding)
>> Thanks!
>>
>> On Thu, Aug 15, 2019 at 11:53 PM Colin McCabe  wrote:
>> >
>> > On Thu, Aug 15, 2019, at 11:47, Jason Gustafson wrote:
>> > > Hey Colin, I think deleting all offsets is equivalent to deleting the
>> > > group, which can be done with the `deleteConsumerGroups` api. I
>> debated
>> > > whether there should be a way to delete partitions for all
>> unsubscribed
>> > > topics, but I decided to start with a simple API.
>> >
>> > That's a fair point-- deleting the group covers the main use-case for
>> deleting all offsets.  So we might as well keep it simple for now.
>> >
>> > cheers,
>> > Colin
>> >
>> > >
>> > > I'm going to close this vote. The final result is +3 with myself,
>> Guozhang,
>> > > and Colin voting.
>> > >
>> > > -Jason
>> > >
>> > > On Tue, Aug 13, 2019 at 9:21 AM Colin McCabe 
>> wrote:
>> > >
>> > > > Hi Jason,
>> > > >
>> > > > Thanks for the KIP.
>> > > >
>> > > > Is there ever a desire to delete all the offsets for a given group?
>> > > > Should the protocol and tools support this?
>> > > >
>> > > > +1 (binding)
>> > > >
>> > > > best,
>> > > > Colin
>> > > >
>> > > >
>> > > > On Mon, Aug 12, 2019, at 10:57, Guozhang Wang wrote:
>> > > > > +1 (binding).
>> > > > >
>> > > > > Thanks Jason!
>> > > > >
>> > > > > On Wed, Aug 7, 2019 at 11:18 AM Jason Gustafson <
>> ja...@confluent.io>
>> > > > wrote:
>> > > > >
>> > > > > > Hi All,
>> > > > > >
>> > > > > > I'd like to start a vote on KIP-496:
>> > > > > >
>> > > > > >
>> > > >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-496%3A+Administrative+API+to+delete+consumer+offsets
>> > > > > > .
>> > > > > > +1
>> > > > > > from me of course.
>> > > > > >
>> > > > > > -Jason
>> > > > > >
>> > > > >
>> > > > >
>> > > > > --
>> > > > > -- Guozhang
>> > > > >
>> > > >
>> > >
>>
>


[VOTE] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-16 Thread David Jacot
Hi all,

I would like to start a vote on KIP-511:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers

Best,
David


Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-16 Thread David Jacot
Hi all,

I have updated the KIP to reflect the offline discussion that we have had.
It seems that we have finally
reached a consensus on the approach. Therefore, I will start a vote.

Best,
David


On Wed, Sep 11, 2019 at 3:53 PM David Jacot  wrote:

> Hi all,
>
> I have discussed with Magnus about the various options to get his view
> from a librdkafka perspective
> and he has suggested a good alternative.
>
> It seems we could continue with the idea to use the
> ApiVersionsRequest/Response but we a different
> failing back strategy. When a broker get an unknown ApiVersionsRequest, it
> could continue to fail back
> to version 0 as today but instead of sending back the UNSUPPORTED_VERSION
> error alone in the
> response, it could also provide at minimum the supported version of the
> ApiVersionsRequest. This way,
> a recent client could leverage that information when the error is received
> and send the correct version
> to the broker instead of failing all the way back to version 0.
>
> This way, we can evolve the ApiVersionsRequest while keeping forward
> compatibility of the Request
> Header. It doesn't add any extra round trip.
>
> Knowing this, I think that putting the information in the
> ApiVersionsRequest remains the best option.
>
> What do you think?
>
> Best,
> David
>
> On Tue, Sep 10, 2019 at 1:00 AM Colin McCabe  wrote:
>
>> Hi all,
>>
>> I agree that freezing the request header is not very appealing.  We might
>> want to add something there later.
>>
>> Having a separate request type is very clean, but as David mentioned, it
>> does add an extra request/response cycle to the time required to get a
>> connection into a usable state.
>>
>> One solution to consider is adding the clientVersion and clientType to
>> the request header as optional (tagged) fields.  This would let us skip the
>> extra round trip.  I don't think it's that much more messy than having a
>> separate request type to set the client version and type.  In both cases,
>> you have to handle connections that set the version later than others, or
>> don't set the version at all (for compatibility).  So the version/type has
>> to be mutable and added after the TCP connection itself is established.
>>
>> best,
>> Colin
>>
>>
>> On Mon, Sep 9, 2019, at 06:11, David Jacot wrote:
>> > Hi Gwen,
>> >
>> > The reasoning behind having the information before the Sasl
>> authentication
>> > was to have the information for troubleshooting purposes. For instance,
>> when
>> > there are errors in the handshake, it would be great to know if it comes
>> > from
>> > a specific buggy clients. I think we could live without though but was
>> nice
>> > to
>> > have.
>> >
>> > Yeah. I agree with you. It seems that evolution of the RequestHeader
>> (RH)
>> > and the
>> > ApiVersionsRequest (AVR) is conjunction is more complicated than I
>> > anticipated.
>> > The evolution if the AVR alone works well but it would make the
>> evolution
>> > of the
>> > RH hard in the future. Please, check my other email to see my thoughts.
>> >
>> > Best,
>> > David
>> >
>> > On Mon, Sep 9, 2019 at 3:18 AM Gwen Shapira  wrote:
>> >
>> > > Hey,
>> > >
>> > > Since modifying ApiVersionsRequest seems to be quite involved, do we
>> > > want to re-examine the rejected option of adding another
>> > > request/response pair? It will add another roundtrip, but Kafka
>> > > already expects client connections to be long-lived, so the overhead
>> > > is probably negligible.
>> > >
>> > > In addition, in the rejected alternatives it says: "It also would
>> > > require to be done before the authentication (TLS AuthN aside) and
>> > > thus requiring specific treatment, similarly to the
>> > > ApiVersionsRequest." - which I don't quite understand. Why do we think
>> > > this has to happen before authenticating?
>> > >
>> > > It sounds like addition another protocol will allow us to keep the
>> > > special assumptions around ApiVersionsRequest and will remove the
>> > > dependency on KIP-482. Which will make KIP-511 much simpler than the
>> > > alternative we are currently discussing.
>> > >
>> > > Gwen
>> > >
>> > > On Fri, Sep 6, 2019 at 3:16 PM Colin McCabe 
>> wrote:
>> > > >
>> > > > Hi David,

Re: [DISCUSS] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-17 Thread David Jacot
Hey Jose,

Yes, we have considered it. Check "Put clientName and clientVersion in the
RequestHeader" in the rejected alternatives.

Best,
David

On Tue, Sep 17, 2019 at 12:57 AM Jose Armando Garcia Sancio <
jsan...@confluent.io> wrote:

> On Mon, Sep 9, 2019 at 4:00 PM Colin McCabe  wrote:
>
> >
> > One solution to consider is adding the clientVersion and clientType to
> the
> > request header as optional (tagged) fields.  This would let us skip the
> > extra round trip.  I don't think it's that much more messy than having a
> > separate request type to set the client version and type.  In both cases,
> > you have to handle connections that set the version later than others, or
> > don't set the version at all (for compatibility).  So the version/type
> has
> > to be mutable and added after the TCP connection itself is established.
>
>
> Hey David,
>
> Did we consider Colin's suggestion of adding this information to every
> request using tagged field? If so can we add a section to the KIP
> documenting the decision?
>
> The HTTP protocol solves a similar problem by using a special header called
> User-Agent. In that field clients encode much more information than just
> client version and type. For example Mozilla uses this to include platform
> version and engine version. E.g.
>
> User-Agent: Mozilla/ () 
> () 
>
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent
>
> Thanks!
>
> -Jose
>


Re: [VOTE] KIP-496: Administrative API to delete consumer offsets

2019-09-17 Thread David Jacot
Hi all,

We haven't included the changes in the command line tool to support the new
API. Therefore,
I would like to amend the current KIP to cover the changes in the
`kafka-consumer-groups`
command line tool. The change is rather small and it does not need to add
any new arguments
to the command line tool. so it doesn't make sense to create a new KIP for
it.

*Proposed API*
kafka-consumer-groups.sh --bootstrap-server  --group
 --topic :
ex: --bootstrap-server localhost:9092 --group my-group --topic topic1
--topic topic2:0,1,2

When partitions not provided, all partitions are used.

What do you think?

Best,
David


On Fri, Sep 13, 2019 at 6:42 PM Colin McCabe  wrote:

> Hi David,
>
> Sounds good.
>
> best,
> Colin
>
>
> On Fri, Sep 13, 2019, at 04:45, David Jacot wrote:
> > Hi all,
> >
> > I would like to do another modification to the proposal. In the proposal,
> > the OffsetDeleteResponse
> > doesn't have a top level error field so I would like to add one. Many
> > errors concern the whole
> > group (e.g. GROUP_ID_NOT_FOUND) so it would be great to have a way to
> > communicate them
> > back to the client without having to set such errors for all the
> requested
> > partitions. It makes the
> > error handling on the client easier and cleaner.
> >
> > *Proposed API with the ErrorCode:*
> > {
> >   "apiKey": 47,
> >   "type": "response",
> >   "name": "OffsetDeleteResponse",
> >   "validVersions": "0",
> >   "fields": [
> > { "name": "ErrorCode", "type": "int16", "versions": "0+",
> >   "about": "The top-level error code, or 0 if there was no error." },
> > { "name": "ThrottleTimeMs",  "type": "int32",  "versions": "0+",
> > "ignorable": true,
> >   "about": "The duration in milliseconds for which the request was
> > throttled due to a quota violation, or zero if the request did not
> violate
> > any quota." },
> > { "name": "Topics", "type": "[]OffsetDeleteResponseTopic",
> "versions":
> > "0+",
> >   "about": "The responses for each topic.", "fields": [
> > { "name": "Name", "type": "string", "versions": "0+", "mapKey":
> > true,
> >   "about": "The topic name." },
> > { "name": "Partitions", "type":
> "[]OffsetDeleteResponsePartition",
> > "versions": "0+",
> >   "about": "The responses for each partition in the topic.",
> > "fields": [
> > { "name": "PartitionIndex", "type": "int32", "versions":
> "0+",
> > "mapKey": true,
> >   "about": "The partition index." },
> > { "name": "ErrorCode", "type": "int16", "versions": "0+",
> >   "about": "The error code, or 0 if there was no error." }
> >   ]
> > }
> >   ]
> > }
> >   ]
> > }
> >
> > I would like to know if there are any concerns or objections regarding
> this
> > change before updating the KIP.
> >
> > Best,
> > David
> >
> > On Wed, Sep 4, 2019 at 9:24 AM David Jacot  wrote:
> >
> > > Hi all,
> > >
> > > While implementing the KIP, I have realized that a new error code and
> > > exception is required to notify the caller that offsets of a topic can
> not
> > > be deleted because the group is actively subscribed to the topic.
> > >
> > > I would like to know if there are any concerns with these changes
> before
> > > updating the KIP.
> > >
> > > *Proposed API:*
> > > GROUP_SUBSCRIBED_TO_TOPIC(86, "The consumer group is actively
> subscribed
> > > to the topic", GroupSubscribedToTopicException::new);
> > >
> > > public class GroupSubscribedToTopicException extends ApiException {
> > > public GroupSubscribedToTopicException(String message) {
> > > super(message);
> > > }
> > > }
> > >
> > > Best,
> > > David
> > &

Re: [VOTE] KIP-496: Administrative API to delete consumer offsets

2019-09-18 Thread David Jacot
Indeed, I have forgotten to add the action. There will be a new action «
—delete-offsets ». Sorry!

 *Proposed API*
kafka-consumer-groups.sh --bootstrap-server 
—delete-offsets --group  --topic :
ex: --bootstrap-server localhost:9092 --group my-group --topic
topic1 --topic topic2:0,1,2

When partitions is not provided, all partitions are used.

Best,
David

Le mer. 18 sept. 2019 à 20:09, Colin McCabe  a écrit :

> On Tue, Sep 17, 2019, at 09:07, David Jacot wrote:
> > Hi all,
> >
> > We haven't included the changes in the command line tool to support the
> new
> > API. Therefore,
> > I would like to amend the current KIP to cover the changes in the
> > `kafka-consumer-groups`
> > command line tool. The change is rather small and it does not need to add
> > any new arguments
> > to the command line tool. so it doesn't make sense to create a new KIP
> for
> > it.
> >
> > *Proposed API*
> > kafka-consumer-groups.sh --bootstrap-server  --group
> >  --topic :
> > ex: --bootstrap-server localhost:9092 --group my-group --topic topic1
> > --topic topic2:0,1,2
> >
> > When partitions not provided, all partitions are used.
>
> Hmm.  I think I'm missing something here.  If you try the command you
> specified, you get:
>
> > Command must include exactly one action: --list, --describe, --delete,
> --reset-offsets
>
> Did you mean to add a new action here that was offsets-related?
>
> best,
> Colin
>
> >
> > What do you think?
> >
> > Best,
> > David
> >
> >
> > On Fri, Sep 13, 2019 at 6:42 PM Colin McCabe  wrote:
> >
> > > Hi David,
> > >
> > > Sounds good.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, Sep 13, 2019, at 04:45, David Jacot wrote:
> > > > Hi all,
> > > >
> > > > I would like to do another modification to the proposal. In the
> proposal,
> > > > the OffsetDeleteResponse
> > > > doesn't have a top level error field so I would like to add one. Many
> > > > errors concern the whole
> > > > group (e.g. GROUP_ID_NOT_FOUND) so it would be great to have a way to
> > > > communicate them
> > > > back to the client without having to set such errors for all the
> > > requested
> > > > partitions. It makes the
> > > > error handling on the client easier and cleaner.
> > > >
> > > > *Proposed API with the ErrorCode:*
> > > > {
> > > >   "apiKey": 47,
> > > >   "type": "response",
> > > >   "name": "OffsetDeleteResponse",
> > > >   "validVersions": "0",
> > > >   "fields": [
> > > > { "name": "ErrorCode", "type": "int16", "versions": "0+",
> > > >   "about": "The top-level error code, or 0 if there was no
> error." },
> > > > { "name": "ThrottleTimeMs",  "type": "int32",  "versions": "0+",
> > > > "ignorable": true,
> > > >   "about": "The duration in milliseconds for which the request
> was
> > > > throttled due to a quota violation, or zero if the request did not
> > > violate
> > > > any quota." },
> > > > { "name": "Topics", "type": "[]OffsetDeleteResponseTopic",
> > > "versions":
> > > > "0+",
> > > >   "about": "The responses for each topic.", "fields": [
> > > > { "name": "Name", "type": "string", "versions": "0+",
> "mapKey":
> > > > true,
> > > >   "about": "The topic name." },
> > > > { "name": "Partitions", "type":
> > > "[]OffsetDeleteResponsePartition",
> > > > "versions": "0+",
> > > >   "about": "The responses for each partition in the topic.",
> > > > "fields": [
> > > > { "name": "PartitionIndex", "type": "int32", "versions":
> > > "0+",
> > > > "mapKey": true,
> > > >   "about": "The partiti

Re: [VOTE] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-18 Thread David Jacot
t if they do not.
> >
> > Disconnecting is not really a great error propagation method since it
> > leaves the client oblivious to what went wrong.
> > Instead suggest we return an ApiVersionResponse with an error code and a
> > human-readable error message field.
> >
> >
> >
> > Den ons 18 sep. 2019 kl 20:05 skrev Colin McCabe :
> >
> > > Hi David,
> > >
> > > Thanks for the KIP!
> > >
> > > Nitpick: in the intro paragraph, "Operators of Apache Kafka clusters
> have
> > > literally no information about the clients connected to their clusters"
> > > seems a bit too strong.  We have some information, right?  For
> example, the
> > > client ID, where clients are connecting from, etc.  Maybe it would be
> > > clearer to say "very little information about the type of client
> > > software..."
> > >
> > > Instead of ClientName and ClientVersion, how about ClientSoftwareName
> and
> > > ClientSoftwareVersion?  This might make it clearer what the fields are
> > > for.  I can see people getting confused about the difference between
> > > ClientName and ClientId, which sound pretty similar.  Adding
> "Software" to
> > > the field name makes it much clearer what the difference is between
> these
> > > fields.
> > >
> > > In the "ApiVersions Request/Response Handling" section, it seems like
> > > there is some out-of-date text.  Specifically, it says "we propose to
> add
> > > the supported version of the ApiVersionsRequest in the response sent
> back
> > > to the client alongside the error...".  But on the other hand,
> elsewhere in
> > > the KIP, we say "ApiVersionsResponse is bumped to version 3 but does
> not
> > > have any changes in the schema"  Based on the offline discussion we
> had,
> > > the correct text is the latter (we're not changing
> ApiVersionsRerponse).
> > > So we should remove the text about adding stuff to ApiVersionsResponse.
> > >
> > > In a similar vein, the comment "  // Version 3 is similar to version 2"
> > > should be " // Version 3 is identical to version 2" or something like
> > > that.  Although I guess technically things which are identical are also
> > > similar, the current phrasing could be misleading.
> > >
> > > Now that KIP-482 has been accepted, I think there are a few things that
> > > are worth clarifying in the KIP.  Firstly, ApiVersionsRequest v3
> should be
> > > a "flexible version".  Mainly, that means its request header will
> support
> > > optional tagged fields.  However, ApiVersionsResponse v3 will *not*
> support
> > > optional tagged fields in its response header.  This is necessary
> because--
> > > as you said-- the broker must look at a fixed offset to find the error
> > > code, regardless of the response version.
> > >
> > > I think we should force client software names and versions to follow a
> > > regular expression and disconnect if they do not.  This will prevent
> issues
> > > when using these strings in metrics names.  Probably we want something
> like:
> > >
> > > [\.\-A-Za-z0-9]?[\.\-A-Za-z0-9 ]*[\.\-A-Za-z0-9]?
> > >
> > > Notice this does _not* include underscores, since they get converted to
> > > dots in JMX, causing ambiguity.  It also doesn't allow the first or
> last
> > > character to be a space.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Mon, Sep 16, 2019, at 04:39, Mickael Maison wrote:
> > > > +1 (non binding)
> > > > Thanks for the KIP!
> > > >
> > > > On Mon, Sep 16, 2019 at 12:07 PM David Jacot 
> > > wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > I would like to start a vote on KIP-511:
> > > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers
> > > > >
> > > > > Best,
> > > > > David
> > > >
> > >
> >
>


Re: [VOTE] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-19 Thread David Jacot
Hi all,

I have updated the KIP to incorporate Colin's feedback.

Best,
David

On Thu, Sep 19, 2019 at 8:44 AM David Jacot  wrote:

> Hi Colin,
>
> Thank you for your feedback! Please find my comments/answers below:
>
> *> Nitpick: in the intro paragraph, "Operators of Apache Kafka clusters
> have literally no information about the clients connected to their
> clusters" seems a bit too strong.  We have some information, right?  For
> example, the client ID, where clients are connecting from, etc.  Maybe it
> would be clearer to say "very little information about the type of client
> software..."*
>
> That's fair. I will update it.
>
> *> Instead of ClientName and ClientVersion, how about ClientSoftwareName
> and ClientSoftwareVersion?  This might make it clearer what the fields are
> for.  I can see people getting confused about the difference between
> ClientName and ClientId, which sound pretty similar.  Adding "Software" to
> the field name makes it much clearer what the difference is between these
> fields.*
>
> Good point. I like your suggestion. I will update it.
>
> *> In the "ApiVersions Request/Response Handling" section, it seems like
> there is some out-of-date text.  Specifically, it says "we propose to add
> the supported version of the ApiVersionsRequest in the response sent back
> to the client alongside the error...".  But on the other hand, elsewhere in
> the KIP, we say "ApiVersionsResponse is bumped to version 3 but does not
> have any changes in the schema"  Based on the offline discussion we had,
> the correct text is the latter (we're not changing ApiVersionsRerponse).
> So we should remove the text about adding stuff to ApiVersionsResponse.*
>
> Sorry for the confusion. I think my usage of the word "add" is not
> appropriate here. The ApiVersionsResponse won't change as you said. My
> proposal is to provide the supported versions of the ApiVersionsRequest in
> the response by populating the existing `api_versions` field. In the
> current version, when an error occurs, the `api_versions` is empty in the
> response. Providing it enables the client to re-send the latest version
> supported by the broker instead of defaulting to zero. I will update the
> KIP to make this clearer.
>
> *> In a similar vein, the comment "  // Version 3 is similar to version 2"
> should be " // Version 3 is identical to version 2" or something like
> that.  Although I guess technically things which are identical are also
> similar, the current phrasing could be misleading.*
>
> Good point. I will use `Version 3 is the same as version 2.` which is the
> statement already used in other requests/responses.
>
>
> *> Now that KIP-482 has been accepted, I think there are a few things that
> are worth clarifying in the KIP.  Firstly, ApiVersionsRequest v3 should be
> a "flexible version".  Mainly, that means its request header will support
> optional tagged fields.  However, ApiVersionsResponse v3 will *not* support
> optional tagged fields in its response header.  This is necessary because--
> as you said-- the broker must look at a fixed offset to find the error
> code, regardless of the response version.*
> Right. I have put it because I thought your PR would do it. I will update
> this. By the way, it means that the request/response must be updated to the
> generated ones, isn't it? AVR is still using the old mechanism.
>
>
>
> *> I think we should force client software names and versions to follow a
> regular expression and disconnect if they do not.  This will prevent issues
> when using these strings in metrics names.  Probably we want something
> like:> [\.\-A-Za-z0-9]?[\.\-A-Za-z0-9 ]*[\.\-A-Za-z0-9]?> Notice this does
> _not* include underscores, since they get converted to dots in JMX, causing
> ambiguity.  It also doesn't allow the first or last character to be a
> space.*
>
> I do agree and I have already put something along those lines in the
> proposal. See the "Validation" chapter. I have proposed to use a more
> restrictive validation which does not allow white spaces. I think spaces
> wouldn't be used in software name nor version. Is it OK for you if we stick
> to the more restrictive one? Thank your letting me know about the
> underscores. I have missed this.
>
> Regarding disconnecting when the validation fails, this is what I have
> proposed as well. Magnus has brought a good point though. Using an explicit
> error like `INVALID_REQUEST` may be better. In this case, the client would
> have to disconnect when it happens. I will update the KIP to reflect this.
>
> Best,
> David

Re: [VOTE] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-20 Thread David Jacot
> Thanks for the clarification.  The proposed behavior sounds reasonable.
Can you add a note about the implementation on the client?  The client
needs to be prepared to handle > a response that doesn't include the
versions, as well, since v1 did not.

I have added a note about the implementation in the KIP. In short, when the
client receives an unsupported version, it defaults to version 0. As
version 0 contains both the ErrorCode and the ApiKeys fields, the client
can check the error and in case of UNSUPPORTED_VERSION, it can check the
ApiKeys to get the supported versions. If not present, it default to
version 0.

> Hmm. Like we discussed above, there is a very important difference in the
v3 response, which is that the versions will be included even if the
client's version was higher than
> what the broker supports.  We should add a comment about that.

Yeah. I think the change that we propose to enhance the handling of
unsupported versions of ApiVersionsRequest/Response is orthogonal to the
version bump. Concretely, the versions will be included in the
ApiVersionsResponse v0 - the request/response used by the broker when
failing back - so it is a bit misleading to say that starting from version
3, the broker populate the ApiKeys field with the information about the
supported versions of the AVR. I would rather put a note saying: Starting
from Apache Kafka 2.4, ApiKeys field is populated with the supported
versions of the ApiVersionsRequest when an UNSUPPORTED_VERSION error is
returned. Would this work for you?

> Agreed.  This is a good use-case for INVALID_REQUEST.  We should add a
comment that this is now a valid error.

I have documented the error in the Public Interfaces section.

Best,
David

On Thu, Sep 19, 2019 at 10:52 PM Colin McCabe  wrote:

> On Wed, Sep 18, 2019, at 23:44, David Jacot wrote:
> > Hi Colin,
> >
> > Thank you for your feedback! Please find my comments/answers below:
> >
> > *> Nitpick: in the intro paragraph, "Operators of Apache Kafka clusters
> > have literally no information about the clients connected to their
> > clusters" seems a bit too strong.  We have some information, right?  For
> > example, the client ID, where clients are connecting from, etc.  Maybe it
> > would be clearer to say "very little information about the type of client
> > software..."*
> >
> > That's fair. I will update it.
> >
> > *> Instead of ClientName and ClientVersion, how about ClientSoftwareName
> > and ClientSoftwareVersion?  This might make it clearer what the fields
> are
> > for.  I can see people getting confused about the difference between
> > ClientName and ClientId, which sound pretty similar.  Adding "Software"
> to
> > the field name makes it much clearer what the difference is between these
> > fields.*
> >
> > Good point. I like your suggestion. I will update it.
> >
> > *> In the "ApiVersions Request/Response Handling" section, it seems like
> > there is some out-of-date text.  Specifically, it says "we propose to add
> > the supported version of the ApiVersionsRequest in the response sent back
> > to the client alongside the error...".  But on the other hand, elsewhere
> in
> > the KIP, we say "ApiVersionsResponse is bumped to version 3 but does not
> > have any changes in the schema"  Based on the offline discussion we had,
> > the correct text is the latter (we're not changing ApiVersionsRerponse).
> > So we should remove the text about adding stuff to ApiVersionsResponse.*
> >
> > Sorry for the confusion. I think my usage of the word "add" is not
> > appropriate here. The ApiVersionsResponse won't change as you said. My
> > proposal is to provide the supported versions of the ApiVersionsRequest
> in
> > the response by populating the existing `api_versions` field. In the
> > current version, when an error occurs, the `api_versions` is empty in the
> > response. Providing it enables the client to re-send the latest version
> > supported by the broker instead of defaulting to zero. I will update the
> > KIP to make this clearer.
>
> Thanks for the clarification.  The proposed behavior sounds reasonable.
> Can you add a note about the implementation on the client?  The client
> needs to be prepared to handle a response that doesn't include the
> versions, as well, since v1 did not.
>
> >
> > *> In a similar vein, the comment "  // Version 3 is similar to version
> 2"
> > should be " // Version 3 is identical to version 2" or something like
> > that.  Although I guess technically things which are identical are also
> > similar, the current 

Re: [VOTE] KIP-496: Administrative API to delete consumer offsets

2019-09-20 Thread David Jacot
Great. I have updated the KIP.

Thanks,
David

On Thu, Sep 19, 2019 at 10:56 PM Colin McCabe  wrote:

> Sounds good to me.  It makes sense to add this functionality to the
> command line.
>
> best,
> Colin
>
> On Wed, Sep 18, 2019, at 11:26, David Jacot wrote:
> > Indeed, I have forgotten to add the action. There will be a new action «
> > —delete-offsets ». Sorry!
> >
> >  *Proposed API*
> > kafka-consumer-groups.sh --bootstrap-server 
> > —delete-offsets --group  --topic : numbers>
> > ex: --bootstrap-server localhost:9092 --group my-group --topic
> > topic1 --topic topic2:0,1,2
> >
> > When partitions is not provided, all partitions are used.
> >
> > Best,
> > David
> >
> > Le mer. 18 sept. 2019 à 20:09, Colin McCabe  a
> écrit :
> >
> > > On Tue, Sep 17, 2019, at 09:07, David Jacot wrote:
> > > > Hi all,
> > > >
> > > > We haven't included the changes in the command line tool to support
> the
> > > new
> > > > API. Therefore,
> > > > I would like to amend the current KIP to cover the changes in the
> > > > `kafka-consumer-groups`
> > > > command line tool. The change is rather small and it does not need
> to add
> > > > any new arguments
> > > > to the command line tool. so it doesn't make sense to create a new
> KIP
> > > for
> > > > it.
> > > >
> > > > *Proposed API*
> > > > kafka-consumer-groups.sh --bootstrap-server 
> --group
> > > >  --topic :
> > > > ex: --bootstrap-server localhost:9092 --group my-group --topic topic1
> > > > --topic topic2:0,1,2
> > > >
> > > > When partitions not provided, all partitions are used.
> > >
> > > Hmm.  I think I'm missing something here.  If you try the command you
> > > specified, you get:
> > >
> > > > Command must include exactly one action: --list, --describe,
> --delete,
> > > --reset-offsets
> > >
> > > Did you mean to add a new action here that was offsets-related?
> > >
> > > best,
> > > Colin
> > >
> > > >
> > > > What do you think?
> > > >
> > > > Best,
> > > > David
> > > >
> > > >
> > > > On Fri, Sep 13, 2019 at 6:42 PM Colin McCabe 
> wrote:
> > > >
> > > > > Hi David,
> > > > >
> > > > > Sounds good.
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > >
> > > > > On Fri, Sep 13, 2019, at 04:45, David Jacot wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > I would like to do another modification to the proposal. In the
> > > proposal,
> > > > > > the OffsetDeleteResponse
> > > > > > doesn't have a top level error field so I would like to add one.
> Many
> > > > > > errors concern the whole
> > > > > > group (e.g. GROUP_ID_NOT_FOUND) so it would be great to have a
> way to
> > > > > > communicate them
> > > > > > back to the client without having to set such errors for all the
> > > > > requested
> > > > > > partitions. It makes the
> > > > > > error handling on the client easier and cleaner.
> > > > > >
> > > > > > *Proposed API with the ErrorCode:*
> > > > > > {
> > > > > >   "apiKey": 47,
> > > > > >   "type": "response",
> > > > > >   "name": "OffsetDeleteResponse",
> > > > > >   "validVersions": "0",
> > > > > >   "fields": [
> > > > > > { "name": "ErrorCode", "type": "int16", "versions": "0+",
> > > > > >   "about": "The top-level error code, or 0 if there was no
> > > error." },
> > > > > > { "name": "ThrottleTimeMs",  "type": "int32",  "versions":
> "0+",
> > > > > > "ignorable": true,
> > > > > >   "about": "The duration in milliseconds for which the
> request
> > > was
> > > > > > throttled due to a quota violation, or zero if the

Re: [VOTE] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-23 Thread David Jacot
Hi Jason,

The response will be a flexible version but the response header won't be
(only for the api versions response). I have forgotten to change this point
in the KIP. I will make this point clearer.

I didn't know that INVALID_REQUEST already exists. Yes, it makes sense to
reuse it then.

Best,
David

On Sat, Sep 21, 2019 at 3:02 AM Kevin Lu  wrote:

> +1 (non-binding)
>
> Definitely needed this before as it would have saved me some time from
> trying to guess a client's version from api version/source code. Thanks for
> the KIP!
>
> Regards,
> Kevin
>
> On Fri, Sep 20, 2019 at 4:29 PM Jason Gustafson 
> wrote:
>
> > +1 from me. This is a clever solution. Kind of a pity we couldn't work
> > flexible version support into the response, but I understand why it is
> > difficult.
> >
> > One minor nitpick: the INVALID_REQUEST error already exists. Are you
> > intending to reuse it?
> >
> > Thanks,
> > Jason
> >
> > On Fri, Sep 20, 2019 at 3:50 PM Konstantine Karantasis <
> > konstant...@confluent.io> wrote:
> >
> > > Quite useful KIP from an operational standpoint and I also like it in
> its
> > > most recent revised form.
> > >
> > > +1 (non-binding).
> > >
> > > Konstantine
> > >
> > >
> > > On Fri, Sep 20, 2019 at 9:55 AM Gwen Shapira 
> wrote:
> > >
> > > > +1 (binding)
> > > >
> > > > Thanks for the KIP, David and for the help with the design, Colin. I
> > > > think it looks great now.
> > > >
> > > > On Fri, Sep 20, 2019 at 9:42 AM Colin McCabe 
> > wrote:
> > > > >
> > > > > On Fri, Sep 20, 2019, at 01:45, David Jacot wrote:
> > > > > > > Thanks for the clarification.  The proposed behavior sounds
> > > > reasonable.
> > > > > > > Can you add a note about the implementation on the client?  The
> > > > client
> > > > > > > needs to be prepared to handle > a response that doesn't
> include
> > > the
> > > > > > > versions, as well, since v1 did not.
> > > > > >
> > > > > > I have added a note about the implementation in the KIP. In
> short,
> > > > when the
> > > > > > client receives an unsupported version, it defaults to version 0.
> > As
> > > > > > version 0 contains both the ErrorCode and the ApiKeys fields, the
> > > > client
> > > > > > can check the error and in case of UNSUPPORTED_VERSION, it can
> > check
> > > > the
> > > > > > ApiKeys to get the supported versions. If not present, it default
> > to
> > > > > > version 0.
> > > > > >
> > > > > > > Hmm. Like we discussed above, there is a very important
> > difference
> > > > in the
> > > > > > > v3 response, which is that the versions will be included even
> if
> > > the
> > > > > > > client's version was higher than what the broker supports.
> > > > > > > We should add a comment about that.
> > > > > >
> > > > > > Yeah. I think the change that we propose to enhance the handling
> of
> > > > > > unsupported versions of ApiVersionsRequest/Response is orthogonal
> > to
> > > > the
> > > > > > version bump. Concretely, the versions will be included in the
> > > > > > ApiVersionsResponse v0 - the request/response used by the broker
> > when
> > > > > > failing back - so it is a bit misleading to say that starting
> from
> > > > version
> > > > > > 3, the broker populate the ApiKeys field with the information
> about
> > > the
> > > > > > supported versions of the AVR. I would rather put a note saying:
> > > > Starting
> > > > > > from Apache Kafka 2.4, ApiKeys field is populated with the
> > supported
> > > > > > versions of the ApiVersionsRequest when an UNSUPPORTED_VERSION
> > error
> > > is
> > > > > > returned. Would this work for you?
> > > > >
> > > > > Thanks for the clarification.  Yes, that makes sense.  Adding the
> > > > additional fields doesn't need to be tied to the version of
> > > > ApiVersionsResponse.
> > > > >
> > > > > Keep in mind, though, that you still have to handl

Re: [VOTE] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-09-23 Thread David Jacot
Hi all,

The vote has passed with +3 binding votes (Colin McCabe, Gwen Shapira,
Jason Gustafson) and +3 non binding votes (Mickael Maison, Konstantine
Karantasis, Kevin Lu). \o/

Thanks to everyone that reviewed and helped improve this proposal, and
huge thanks to Colin for his great feedback.

Best,
David

On Mon, Sep 23, 2019 at 9:28 AM David Jacot  wrote:

> Hi Jason,
>
> The response will be a flexible version but the response header won't be
> (only for the api versions response). I have forgotten to change this point
> in the KIP. I will make this point clearer.
>
> I didn't know that INVALID_REQUEST already exists. Yes, it makes sense to
> reuse it then.
>
> Best,
> David
>
> On Sat, Sep 21, 2019 at 3:02 AM Kevin Lu  wrote:
>
>> +1 (non-binding)
>>
>> Definitely needed this before as it would have saved me some time from
>> trying to guess a client's version from api version/source code. Thanks
>> for
>> the KIP!
>>
>> Regards,
>> Kevin
>>
>> On Fri, Sep 20, 2019 at 4:29 PM Jason Gustafson 
>> wrote:
>>
>> > +1 from me. This is a clever solution. Kind of a pity we couldn't work
>> > flexible version support into the response, but I understand why it is
>> > difficult.
>> >
>> > One minor nitpick: the INVALID_REQUEST error already exists. Are you
>> > intending to reuse it?
>> >
>> > Thanks,
>> > Jason
>> >
>> > On Fri, Sep 20, 2019 at 3:50 PM Konstantine Karantasis <
>> > konstant...@confluent.io> wrote:
>> >
>> > > Quite useful KIP from an operational standpoint and I also like it in
>> its
>> > > most recent revised form.
>> > >
>> > > +1 (non-binding).
>> > >
>> > > Konstantine
>> > >
>> > >
>> > > On Fri, Sep 20, 2019 at 9:55 AM Gwen Shapira 
>> wrote:
>> > >
>> > > > +1 (binding)
>> > > >
>> > > > Thanks for the KIP, David and for the help with the design, Colin. I
>> > > > think it looks great now.
>> > > >
>> > > > On Fri, Sep 20, 2019 at 9:42 AM Colin McCabe 
>> > wrote:
>> > > > >
>> > > > > On Fri, Sep 20, 2019, at 01:45, David Jacot wrote:
>> > > > > > > Thanks for the clarification.  The proposed behavior sounds
>> > > > reasonable.
>> > > > > > > Can you add a note about the implementation on the client?
>> The
>> > > > client
>> > > > > > > needs to be prepared to handle > a response that doesn't
>> include
>> > > the
>> > > > > > > versions, as well, since v1 did not.
>> > > > > >
>> > > > > > I have added a note about the implementation in the KIP. In
>> short,
>> > > > when the
>> > > > > > client receives an unsupported version, it defaults to version
>> 0.
>> > As
>> > > > > > version 0 contains both the ErrorCode and the ApiKeys fields,
>> the
>> > > > client
>> > > > > > can check the error and in case of UNSUPPORTED_VERSION, it can
>> > check
>> > > > the
>> > > > > > ApiKeys to get the supported versions. If not present, it
>> default
>> > to
>> > > > > > version 0.
>> > > > > >
>> > > > > > > Hmm. Like we discussed above, there is a very important
>> > difference
>> > > > in the
>> > > > > > > v3 response, which is that the versions will be included even
>> if
>> > > the
>> > > > > > > client's version was higher than what the broker supports.
>> > > > > > > We should add a comment about that.
>> > > > > >
>> > > > > > Yeah. I think the change that we propose to enhance the
>> handling of
>> > > > > > unsupported versions of ApiVersionsRequest/Response is
>> orthogonal
>> > to
>> > > > the
>> > > > > > version bump. Concretely, the versions will be included in the
>> > > > > > ApiVersionsResponse v0 - the request/response used by the broker
>> > when
>> > > > > > failing back - so it is a bit misleading to say that starting
>> from
>> > > > version
>> > > > > > 3, the broker populate the Ap

Re: [VOTE] KIP-511: Collect and Expose Client's Name and Version in the Brokers

2019-10-14 Thread David Jacot
Hi all,

Jun,
The new fields are not flexible fields while the request and response are
flexible versions. It does not cost us because the version of the request
is bumped anyway to enable the flexible versions. The rationale behind this
choice is that it forces the clients to provide the information.

Guozhang,
"Connections" is the name of the metric and it does not change. Concerns
regarding the scalability of this metric have been expressed during the
implementation so we won't implement it. The idea was to allow operators to
list all the active connections via JMX. I am looking for alternatives at
this point.

Best,
David


On Sat, Oct 12, 2019 at 1:15 AM Harsha Chintalapani  wrote:

> +1 (binding). Thanks for the KIP this is going to be super useful.
>
> Thanks,
> Harsha
>
> On Fri, Oct 11, 2019 at 2:57 PM Guozhang Wang  wrote:
>
> > Hi David,
> >
> > Thanks for the KIP. I know I'm late on voting here but I'd be +1 on this
> > too!
> >
> > Just a quick clarification on the tag "name=Connections" of the third
> > metric, what would be the format of "Connections" here? Would that be the
> > connection listener name?
> >
> >
> > Guozhang
> >
> >
> > On Fri, Oct 11, 2019 at 1:49 PM Jun Rao  wrote:
> >
> > > Hi, David,
> > >
> > > Thanks for the KIP. Just a minor comment below.
> > >
> > > 100. It seems that the new flexible fields need tag numbers. Could you
> > add
> > > them to the wiki?
> > >
> > > Jun
> > >
> > > On Mon, Sep 23, 2019 at 11:37 AM Jason Gustafson 
> > > wrote:
> > >
> > > > Thanks David for the clarification. That sounds good.
> > > >
> > > > On Mon, Sep 23, 2019 at 12:35 AM David Jacot 
> > > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > The vote has passed with +3 binding votes (Colin McCabe, Gwen
> > Shapira,
> > > > > Jason Gustafson) and +3 non binding votes (Mickael Maison,
> > Konstantine
> > > > > Karantasis, Kevin Lu). \o/
> > > > >
> > > > > Thanks to everyone that reviewed and helped improve this proposal,
> > and
> > > > > huge thanks to Colin for his great feedback.
> > > > >
> > > > > Best,
> > > > > David
> > > > >
> > > > > On Mon, Sep 23, 2019 at 9:28 AM David Jacot 
> > > wrote:
> > > > >
> > > > > > Hi Jason,
> > > > > >
> > > > > > The response will be a flexible version but the response header
> > won't
> > > > be
> > > > > > (only for the api versions response). I have forgotten to change
> > this
> > > > > point
> > > > > > in the KIP. I will make this point clearer.
> > > > > >
> > > > > > I didn't know that INVALID_REQUEST already exists. Yes, it makes
> > > sense
> > > > to
> > > > > > reuse it then.
> > > > > >
> > > > > > Best,
> > > > > > David
> > > > > >
> > > > > > On Sat, Sep 21, 2019 at 3:02 AM Kevin Lu 
> > > > wrote:
> > > > > >
> > > > > >> +1 (non-binding)
> > > > > >>
> > > > > >> Definitely needed this before as it would have saved me some
> time
> > > from
> > > > > >> trying to guess a client's version from api version/source code.
> > > > Thanks
> > > > > >> for
> > > > > >> the KIP!
> > > > > >>
> > > > > >> Regards,
> > > > > >> Kevin
> > > > > >>
> > > > > >> On Fri, Sep 20, 2019 at 4:29 PM Jason Gustafson <
> > ja...@confluent.io
> > > >
> > > > > >> wrote:
> > > > > >>
> > > > > >> > +1 from me. This is a clever solution. Kind of a pity we
> > couldn't
> > > > work
> > > > > >> > flexible version support into the response, but I understand
> why
> > > it
> > > > is
> > > > > >> > difficult.
> > > > > >> >
> > > > > >> > One minor nitpick: the INVALID_REQUEST error already exists.
> Are
> > > you
> > > > > >> > intending to reuse it?
&g

Re: [DISCUSS] KIP-531: Drop support for Scala 2.11 in Kafka 2.5

2019-11-08 Thread David Jacot
Thanks for the KIP. It is a big +1 from me.

David

On Fri, Nov 8, 2019 at 2:56 AM Gwen Shapira  wrote:

> About time :) Let's do it.
>
> On Thu, Nov 7, 2019 at 5:35 PM Ismael Juma  wrote:
> >
> > Hi all,
> >
> > I think it's time to simplify our development environment by dropping
> > support for Scala 2.11. Please take a look at the proposal and
> > provide feedback:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-531%3A+Drop+support+for+Scala+2.11+in+Kafka+2.5
> >
> > Ismael
>


Re: [ANNOUNCE] New committer: Mickael Maison

2019-11-08 Thread David Jacot
Congrats Mickeal, well deserved!

On Fri, Nov 8, 2019 at 8:56 AM Tom Bentley  wrote:

> Congratulations Mickael!
>
> On Fri, Nov 8, 2019 at 6:41 AM Vahid Hashemian 
> wrote:
>
> > Congrats Mickael,
> >
> > Well deserved!
> >
> > --Vahid
> >
> > On Thu, Nov 7, 2019 at 9:10 PM Maulin Vasavada <
> maulin.vasav...@gmail.com>
> > wrote:
> >
> > > Congratulations Mickael!
> > >
> > > On Thu, Nov 7, 2019 at 8:27 PM Manikumar 
> > > wrote:
> > >
> > > > Congrats Mickeal!
> > > >
> > > > On Fri, Nov 8, 2019 at 9:05 AM Dong Lin  wrote:
> > > >
> > > > > Congratulations Mickael!
> > > > >
> > > > > On Thu, Nov 7, 2019 at 1:38 PM Jun Rao  wrote:
> > > > >
> > > > > > Hi, Everyone,
> > > > > >
> > > > > > The PMC of Apache Kafka is pleased to announce a new Kafka
> > committer
> > > > > > Mickael
> > > > > > Maison.
> > > > > >
> > > > > > Mickael has been contributing to Kafka since 2016. He proposed
> and
> > > > > > implemented multiple KIPs. He has also been propomating Kafka
> > through
> > > > > blogs
> > > > > > and public talks.
> > > > > >
> > > > > > Congratulations, Mickael!
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jun (on behalf of the Apache Kafka PMC)
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> > --
> >
> > Thanks!
> > --Vahid
> >
>


Re: [VOTE] KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations

2020-06-09 Thread David Jacot
Hi Jun,

Both are already in the KIP, see "New Broker Configurations" chapter. I
think
that we need them in order to be able to define different burst for the new
quota.

Best,
David

On Tue, Jun 9, 2020 at 7:48 PM Jun Rao  wrote:

> Hi, David,
>
> Another thing. Should we add controller.quota.window.size.seconds and
> controller.quota.window.num
> or just reuse the existing quota.window.size.seconds and quota.window.num
> that are used for other types of quotas?
>
> Thanks,
>
> Jun
>
> On Tue, Jun 9, 2020 at 10:30 AM Jun Rao  wrote:
>
> > Hi, David,
> >
> > Thanks for the KIP. The name QUOTA_VIOLATED sounds reasonable to me. +1
> on
> > the KIP.
> >
> > Jun
> >
> > On Tue, Jun 9, 2020 at 5:07 AM David Jacot  wrote:
> >
> >> Hi Colin,
> >>
> >> Thank you for your feedback.
> >>
> >> Jun has summarized the situation pretty well. Thanks Jun! I would like
> to
> >> complement it with the following points:
> >>
> >> 1. Indeed, when the quota is exceeded, the broker will reject the topic
> >> creations, partition creations and topics deletions that are exceeding
> >> with the new QUOTA_VIOLATED error. The ThrottleTimeMs field will
> >> be populated accordingly to let the client know how long it must wait.
> >>
> >> 2. I do agree that we actually want a mechanism to apply back pressure
> >> to the clients. The KIP basically proposes a mechanism to control and to
> >> limit the rate of operations before entering the controller. I think
> that
> >> it is
> >> similar to your thinking but is enforced based on a defined quota
> instead
> >> of relying on the number of pending items in the controller.
> >>
> >> 3. You mentioned an alternative idea in your comments that, if I
> >> understood
> >> correctly, would bound the queue to limit the overload and reject work
> if
> >> the
> >> queue is full. I have been thinking about this as well but I don't think
> >> that it
> >> works well in our case.
> >> - The first reason is the one mentioned by Jun. We actually want to be
> >> able
> >> to limit specific clients (the misbehaving ones) in a multi-tenant
> >> environment.
> >> - The second reason is that, at least in our current implementation, the
> >> length of
> >> the queue is not really a good characteristic to estimate the load.
> >> Coming back
> >> to your example of the CreateTopicsRequest. They create path in ZK for
> >> each
> >> newly created topics which trigger a ChangeTopic event in the
> controller.
> >> That
> >> single event could be for a single topic in some cases or for a thousand
> >> topics
> >> in others.
> >> These two reasons aside, bounding the queue also introduces a knob which
> >> requires some tuning and thus suffers from all the points you mentioned
> as
> >> well, isn't it? The value may be true for one version but not for
> another.
> >>
> >> 4. Regarding the integration with KIP-500. The implementation of this
> KIP
> >> will span across the ApiLayer and the AdminManager. To be honest, we
> >> can influence the implementation to work best with what you have in mind
> >> for the future controller. If you could shed some more light on the
> future
> >> internal architecture, I can take this into account during the
> >> implementation.
> >>
> >> 5. Regarding KIP-590. For the create topics request, create partitions
> >> request,
> >> and delete topics request, we are lucky enough to have directed all of
> >> them
> >> to
> >> the controller already so we are fine with doing the admission in the
> >> broker
> >> which hosts the controller. If we were to throttle more operations in
> the
> >> future,
> >> I believe that we can continue to do it centrally while leveraging the
> >> principal
> >> that is forwarded to account for the right tenant. The reason why I
> would
> >> like
> >> to keep it central is that we are talking about sparse (or bursty)
> >> workloads here
> >> so distributing the quota among all the brokers makes little sense.
> >>
> >> 6. Regarding the naming of the new error code. BUSY sounds too generic
> to
> >> me so I would rather prefer a specific error code. The main reason being
> >> that
> >> we may be able to reuse it in the future for other quotas. That being
> >&

Re: [VOTE] KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations

2020-06-10 Thread David Jacot
Hi Colin and Jun,

I have no problem if we have to rewrite part of it when the new controller
comes
out. I will be more than happy to help out.

Regarding KIP-590, I think that we can cope with a principal as a string
when the
time comes. The user entity name is defined with a string already.

Regarding the name of the error, you have made a good point. I do agree
that it
is important to differentiate the two cases. I propose the following two
errors:
- THROTTLING_QUOTA_EXCEEDED - Throttling is slightly better than rate as
we have quotas which are not rate (e.g. request quota). This one is
retryable
once the throttle time is passed.
- LIMIT_QUOTA_EXCEEDED - This one would indicate that the limit has been
reached and is a final error.
We only need the former in this KIP. What do you think?

Jun, I have added a few examples in the KIP. The new name works exactly like
the existing one once it is added to the accepted dynamic configs for the
user
and the client entities. I have added a "Kafka Config Command" chapter in
the
KIP. I will also open a Jira to not forget updating the AK documentation
once
the KIP gets merged.

Thanks,
David

On Wed, Jun 10, 2020 at 3:03 AM Jun Rao  wrote:

> Hi, Colin,
>
> Good point. Maybe sth like THROTTLING_QUOTA_VIOLATED will make this clear.
>
> Hi, David,
>
> We added a new quota name in the KIP. You chose not to bump up the version
> of DESCRIBE_CLIENT_QUOTAS and ALTER_CLIENT_QUOTAS, which seems ok since the
> quota name is represented as a string. However, the new quota name can be
> used in client tools for setting and listing the quota (
> https://kafka.apache.org/documentation/#quotas). Could you document how
> the
> new name will be used in those tools?
>
> Thanks,
>
> Jun
>
> On Tue, Jun 9, 2020 at 3:37 PM Colin McCabe  wrote:
>
> > On Tue, Jun 9, 2020, at 05:06, David Jacot wrote:
> > > Hi Colin,
> > >
> > > Thank you for your feedback.
> > >
> > > Jun has summarized the situation pretty well. Thanks Jun! I would like
> to
> > > complement it with the following points:
> > >
> > > 1. Indeed, when the quota is exceeded, the broker will reject the topic
> > > creations, partition creations and topics deletions that are exceeding
> > > with the new QUOTA_VIOLATED error. The ThrottleTimeMs field will
> > > be populated accordingly to let the client know how long it must wait.
> > >
> > > 2. I do agree that we actually want a mechanism to apply back pressure
> > > to the clients. The KIP basically proposes a mechanism to control and
> to
> > > limit the rate of operations before entering the controller. I think
> that
> > > it is similar to your thinking but is enforced based on a defined
> > > instead of relying on the number of pending items in the controller.
> > >
> > > 3. You mentioned an alternative idea in your comments that, if I
> > understood
> > > correctly, would bound the queue to limit the overload and reject work
> if
> > > the queue is full. I have been thinking about this as well but I don't
> > think
> > > that it  works well in our case.
> > > - The first reason is the one mentioned by Jun. We actually want to be
> > able
> > > to limit specific clients (the misbehaving ones) in a multi-tenant
> > > environment.
> > > - The second reason is that, at least in our current implementation,
> the
> > > length of the queue is not really a good characteristic to estimate the
> > load.
> > > Coming back to your example of the CreateTopicsRequest. They create
> path
> > >  in ZK for each newly created topics which trigger a ChangeTopic event
> > in
> > > the controller. That single event could be for a single topic in some
> > cases or
> > > for a thousand topics in others.
> > > These two reasons aside, bounding the queue also introduces a knob
> which
> > > requires some tuning and thus suffers from all the points you mentioned
> > as
> > > well, isn't it? The value may be true for one version but not for
> > another.
> > >
> > > 4. Regarding the integration with KIP-500. The implementation of this
> KIP
> > > will span across the ApiLayer and the AdminManager. To be honest, we
> > > can influence the implementation to work best with what you have in
> mind
> > > for the future controller. If you could shed some more light on the
> > future
> > > internal architecture, I can take this into account during the
> > > implementation.
> > >
> >
> > Hi David,
> >
> > Good question.  The approach we are thinki

Re: [VOTE] KIP-597: MirrorMaker2 internal topics Formatters

2020-06-10 Thread David Jacot
Hi Mickael,

Thanks for the KIP. That looks very useful.

I have few small comments/suggestions:

1. I was about the make a similar suggestion than Konstantine did regarding
requiring to recompile old formatters. While the formatters are not
directly part of the public API, I think that we can argue that, as they
are accepted by the console consumer, they are somehow part of it. It is a
bit a gray zone. With this in mind, I lean towards supporting both
interfaces by instantiating one or the other instead of making the old one
implements the new one. It is nicer for our users and require a similar
effort to implement overall.

2. Should the interface implement Configurable and Closable with default
empty implementations? That would make it similar to our other interfaces
but I am not entirely sure that works with the properties.

3. Should we directly move the existing Formatters to using the new
interface?

Regards,
David





Le mer. 10 juin 2020 à 19:13, Maulin Vasavada  a
écrit :

> +1 (non-binding)
>
> On Wed, Jun 10, 2020 at 9:47 AM Mickael Maison 
> wrote:
>
> > Bumping this thread. Let me know if you have any questions or feedback.
> >
> > So far, we have 2 binding and 5 non-binding votes.
> >
> > Thanks
> >
> > On Tue, May 19, 2020 at 10:56 AM Manikumar 
> > wrote:
> > >
> > > +1 (binding)
> > >
> > > Thanks for the KIP.
> > >
> > > On Tue, May 19, 2020 at 2:57 PM Mickael Maison <
> mickael.mai...@gmail.com
> > >
> > > wrote:
> > >
> > > > Thanks Konstantine for the feedback (and vote)!
> > > >
> > > > 1) I've added example commands using the new formatters
> > > >
> > > > 2) I updated the Compatiblity section to be more explicit about the
> > > > need for recompilation
> > > >
> > > > 3) Good point, updated
> > > >
> > > > On Tue, May 19, 2020 at 3:18 AM Konstantine Karantasis
> > > >  wrote:
> > > > >
> > > > > Thanks Michael.
> > > > > I think it's useful to enable specialized message formatters by
> > adding
> > > > this
> > > > > interface to the public API.
> > > > >
> > > > > You have my vote: +1 (binding)
> > > > >
> > > > > Just a few optional comments below:
> > > > >
> > > > > 1. Would you mind adding the equivalent command line example in the
> > > > places
> > > > > where you have an example output?
> > > > >
> > > > > Something equivalent to
> > > > > ./bin/kafka-console-consumer.sh --bootstrap-server localhost:9092
> > --topic
> > > > > __consumer_offsets --from-beginning --formatter
> > > > >
> > > >
> >
> "kafka.coordinator.group.GroupMetadataManager\$GroupMetadataMessageFormatter"
> > > > >
> > > > > but with the equivalent formatter classes and expected topic names.
> > > > >
> > > > > 2. I have to note that breaking old formatters by requiring
> > recompilation
> > > > > could be avoided if we didn't change kafka.common.MessageFormatter
> to
> > > > > extend the new org.apache.kafka.common.MessageFormatter. We could
> > > > maintain
> > > > > both, while the old one would remain deprecated and we could
> attempt
> > to
> > > > > instantiate one or the other type when reading the config and use
> > either
> > > > of
> > > > > the two different types in the few places in ConsoleConsumer that a
> > > > > formatter is used. But I admit that for this use case, it's not
> worth
> > > > > maintaining both types. The interface wasn't public before anyways.
> > > > >
> > > > > Given that, my small request would be to rephrase in the
> > compatibility
> > > > > section to say something like:
> > > > > 'Existing MessageFormatters implementations will require no changes
> > > > beyond
> > > > > recompilation.' or similar. Because, to be precise, existing
> > formatters
> > > > > _won't_ work if they are given as a parameter to a 2.6 console
> > consumer,
> > > > > without recompilation as you mention.
> > > > >
> > > > > 3. Finally, a minor comment on skipping the use of the `public`
> > specifier
> > > > > in the interface because it's redundant.
> > > > >
> > > > > Best regards,
> > > > > Konstantine
> > > > >
> > > > > On Mon, May 18, 2020 at 3:26 PM Maulin Vasavada <
> > > > maulin.vasav...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > +1 (non-binding)
> > > > > >
> > > > > > On Mon, May 18, 2020 at 8:49 AM Mickael Maison <
> > > > mickael.mai...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Bumping this thread as KIP freeze is approaching.
> > > > > > >
> > > > > > > It's a pretty small change and I have a PR ready:
> > > > > > > https://github.com/apache/kafka/pull/8604
> > > > > > >
> > > > > > > Thanks
> > > > > > >
> > > > > > > On Mon, May 4, 2020 at 5:26 PM Ryanne Dolan <
> > ryannedo...@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > +1, non-binding
> > > > > > > >
> > > > > > > > On Mon, May 4, 2020, 9:24 AM Christopher Egerton <
> > > > chr...@confluent.io>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > +1 (non-binding)
> > > > > > > > >
> > > > > > > > > On Mon, May 4, 2020 at 5:02 AM Edoardo Comar <
> > eco...@uk.ibm.com>
> > > > > > > w

Re: [VOTE] KIP-597: MirrorMaker2 internal topics Formatters

2020-06-11 Thread David Jacot
Hi Mickael,

Thanks for the updated KIP.

1) I don’t feel too strong about this. I understand your point of view.

2) I just looked at the updated interface and I find a little weird to
already have a deprecated method in a new interface, don’t you? If we
believe that using Configurable is a good thing, it may be better to revise
our position regarding 1) and get rid of that deprecated init method.
Otherwise, it may be better to not implement Configurable after all. I
leave this up to you.

3) Thanks for the clarification.

Best,
David

Le jeu. 11 juin 2020 à 16:34, Mickael Maison  a
écrit :

> Jun,
> Yes I'm aware this is allowed but I feel like it's a bit cheating =)
> Anyway it's a relatively minor KIP so let's do it, +1 binding
>
> David,
> 1) I understand compatibility is important but I feel like supporting
> both the old and new interface is excessive. The console tool does not
> mention the interface and only says a class can be passed so it's not
> really exposed.
> 2) I think it's a good idea, I'll update the KIP
> 3) Yes this is the plan and what I did in the PR. I can see it's not
> explicitly said in the KIP, I'll fix that
>
> Thanks
>
> On Wed, Jun 10, 2020 at 8:24 PM David Jacot  wrote:
> >
> > Hi Mickael,
> >
> > Thanks for the KIP. That looks very useful.
> >
> > I have few small comments/suggestions:
> >
> > 1. I was about the make a similar suggestion than Konstantine did
> regarding
> > requiring to recompile old formatters. While the formatters are not
> > directly part of the public API, I think that we can argue that, as they
> > are accepted by the console consumer, they are somehow part of it. It is
> a
> > bit a gray zone. With this in mind, I lean towards supporting both
> > interfaces by instantiating one or the other instead of making the old
> one
> > implements the new one. It is nicer for our users and require a similar
> > effort to implement overall.
> >
> > 2. Should the interface implement Configurable and Closable with default
> > empty implementations? That would make it similar to our other interfaces
> > but I am not entirely sure that works with the properties.
> >
> > 3. Should we directly move the existing Formatters to using the new
> > interface?
> >
> > Regards,
> > David
> >
> >
> >
> >
> >
> > Le mer. 10 juin 2020 à 19:13, Maulin Vasavada 
> a
> > écrit :
> >
> > > +1 (non-binding)
> > >
> > > On Wed, Jun 10, 2020 at 9:47 AM Mickael Maison <
> mickael.mai...@gmail.com>
> > > wrote:
> > >
> > > > Bumping this thread. Let me know if you have any questions or
> feedback.
> > > >
> > > > So far, we have 2 binding and 5 non-binding votes.
> > > >
> > > > Thanks
> > > >
> > > > On Tue, May 19, 2020 at 10:56 AM Manikumar <
> manikumar.re...@gmail.com>
> > > > wrote:
> > > > >
> > > > > +1 (binding)
> > > > >
> > > > > Thanks for the KIP.
> > > > >
> > > > > On Tue, May 19, 2020 at 2:57 PM Mickael Maison <
> > > mickael.mai...@gmail.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Thanks Konstantine for the feedback (and vote)!
> > > > > >
> > > > > > 1) I've added example commands using the new formatters
> > > > > >
> > > > > > 2) I updated the Compatiblity section to be more explicit about
> the
> > > > > > need for recompilation
> > > > > >
> > > > > > 3) Good point, updated
> > > > > >
> > > > > > On Tue, May 19, 2020 at 3:18 AM Konstantine Karantasis
> > > > > >  wrote:
> > > > > > >
> > > > > > > Thanks Michael.
> > > > > > > I think it's useful to enable specialized message formatters by
> > > > adding
> > > > > > this
> > > > > > > interface to the public API.
> > > > > > >
> > > > > > > You have my vote: +1 (binding)
> > > > > > >
> > > > > > > Just a few optional comments below:
> > > > > > >
> > > > > > > 1. Would you mind adding the equivalent command line example
> in the
> > > > > > places
> > > > > > > where you have an example output?
> > > 

Re: [VOTE] KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations

2020-06-11 Thread David Jacot
Colin, Jun,

Do the proposed error code and the updated KIP look good to you guys? I’d
like to wrap up and close the vote.

Thanks,
David

Le mer. 10 juin 2020 à 14:50, David Jacot  a écrit :

> Hi Colin and Jun,
>
> I have no problem if we have to rewrite part of it when the new controller
> comes
> out. I will be more than happy to help out.
>
> Regarding KIP-590, I think that we can cope with a principal as a string
> when the
> time comes. The user entity name is defined with a string already.
>
> Regarding the name of the error, you have made a good point. I do agree
> that it
> is important to differentiate the two cases. I propose the following two
> errors:
> - THROTTLING_QUOTA_EXCEEDED - Throttling is slightly better than rate as
> we have quotas which are not rate (e.g. request quota). This one is
> retryable
> once the throttle time is passed.
> - LIMIT_QUOTA_EXCEEDED - This one would indicate that the limit has been
> reached and is a final error.
> We only need the former in this KIP. What do you think?
>
> Jun, I have added a few examples in the KIP. The new name works exactly
> like
> the existing one once it is added to the accepted dynamic configs for the
> user
> and the client entities. I have added a "Kafka Config Command" chapter in
> the
> KIP. I will also open a Jira to not forget updating the AK documentation
> once
> the KIP gets merged.
>
> Thanks,
> David
>
> On Wed, Jun 10, 2020 at 3:03 AM Jun Rao  wrote:
>
>> Hi, Colin,
>>
>> Good point. Maybe sth like THROTTLING_QUOTA_VIOLATED will make this clear.
>>
>> Hi, David,
>>
>> We added a new quota name in the KIP. You chose not to bump up the version
>> of DESCRIBE_CLIENT_QUOTAS and ALTER_CLIENT_QUOTAS, which seems ok since
>> the
>> quota name is represented as a string. However, the new quota name can be
>> used in client tools for setting and listing the quota (
>> https://kafka.apache.org/documentation/#quotas). Could you document how
>> the
>> new name will be used in those tools?
>>
>> Thanks,
>>
>> Jun
>>
>> On Tue, Jun 9, 2020 at 3:37 PM Colin McCabe  wrote:
>>
>> > On Tue, Jun 9, 2020, at 05:06, David Jacot wrote:
>> > > Hi Colin,
>> > >
>> > > Thank you for your feedback.
>> > >
>> > > Jun has summarized the situation pretty well. Thanks Jun! I would
>> like to
>> > > complement it with the following points:
>> > >
>> > > 1. Indeed, when the quota is exceeded, the broker will reject the
>> topic
>> > > creations, partition creations and topics deletions that are exceeding
>> > > with the new QUOTA_VIOLATED error. The ThrottleTimeMs field will
>> > > be populated accordingly to let the client know how long it must wait.
>> > >
>> > > 2. I do agree that we actually want a mechanism to apply back pressure
>> > > to the clients. The KIP basically proposes a mechanism to control and
>> to
>> > > limit the rate of operations before entering the controller. I think
>> that
>> > > it is similar to your thinking but is enforced based on a defined
>> > > instead of relying on the number of pending items in the controller.
>> > >
>> > > 3. You mentioned an alternative idea in your comments that, if I
>> > understood
>> > > correctly, would bound the queue to limit the overload and reject
>> work if
>> > > the queue is full. I have been thinking about this as well but I don't
>> > think
>> > > that it  works well in our case.
>> > > - The first reason is the one mentioned by Jun. We actually want to be
>> > able
>> > > to limit specific clients (the misbehaving ones) in a multi-tenant
>> > > environment.
>> > > - The second reason is that, at least in our current implementation,
>> the
>> > > length of the queue is not really a good characteristic to estimate
>> the
>> > load.
>> > > Coming back to your example of the CreateTopicsRequest. They create
>> path
>> > >  in ZK for each newly created topics which trigger a ChangeTopic event
>> > in
>> > > the controller. That single event could be for a single topic in some
>> > cases or
>> > > for a thousand topics in others.
>> > > These two reasons aside, bounding the queue also introduces a knob
>> which
>> > > requires some tuning and thus suffers from all the points you
>> mentioned
>> > as
>> > > well, isn

Re: [VOTE] KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations

2020-06-15 Thread David Jacot
Hi all,

The vote has passed with 5 binding votes (Gwen, Rajini, Mickael, Jun and
Colin)
and 2 non-binding votes (Tom, Anna).

Thank you all for the fruitful discussion! I'd like to particularly thank
Anna who has
heavily contributed to the design of this KIP.

Regards,
David

On Fri, Jun 12, 2020 at 10:08 PM Colin McCabe  wrote:

> +1.  Thanks, David!
>
> best,
> Colin
>
> On Thu, Jun 11, 2020, at 23:51, David Jacot wrote:
> > Colin, Jun,
> >
> > Do the proposed error code and the updated KIP look good to you guys? I’d
> > like to wrap up and close the vote.
> >
> > Thanks,
> > David
> >
> > Le mer. 10 juin 2020 à 14:50, David Jacot  a écrit
> :
> >
> > > Hi Colin and Jun,
> > >
> > > I have no problem if we have to rewrite part of it when the new
> controller
> > > comes
> > > out. I will be more than happy to help out.
> > >
> > > Regarding KIP-590, I think that we can cope with a principal as a
> string
> > > when the
> > > time comes. The user entity name is defined with a string already.
> > >
> > > Regarding the name of the error, you have made a good point. I do agree
> > > that it
> > > is important to differentiate the two cases. I propose the following
> two
> > > errors:
> > > - THROTTLING_QUOTA_EXCEEDED - Throttling is slightly better than rate
> as
> > > we have quotas which are not rate (e.g. request quota). This one is
> > > retryable
> > > once the throttle time is passed.
> > > - LIMIT_QUOTA_EXCEEDED - This one would indicate that the limit has
> been
> > > reached and is a final error.
> > > We only need the former in this KIP. What do you think?
> > >
> > > Jun, I have added a few examples in the KIP. The new name works exactly
> > > like
> > > the existing one once it is added to the accepted dynamic configs for
> the
> > > user
> > > and the client entities. I have added a "Kafka Config Command" chapter
> in
> > > the
> > > KIP. I will also open a Jira to not forget updating the AK
> documentation
> > > once
> > > the KIP gets merged.
> > >
> > > Thanks,
> > > David
> > >
> > > On Wed, Jun 10, 2020 at 3:03 AM Jun Rao  wrote:
> > >
> > >> Hi, Colin,
> > >>
> > >> Good point. Maybe sth like THROTTLING_QUOTA_VIOLATED will make this
> clear.
> > >>
> > >> Hi, David,
> > >>
> > >> We added a new quota name in the KIP. You chose not to bump up the
> version
> > >> of DESCRIBE_CLIENT_QUOTAS and ALTER_CLIENT_QUOTAS, which seems ok
> since
> > >> the
> > >> quota name is represented as a string. However, the new quota name
> can be
> > >> used in client tools for setting and listing the quota (
> > >> https://kafka.apache.org/documentation/#quotas). Could you document
> how
> > >> the
> > >> new name will be used in those tools?
> > >>
> > >> Thanks,
> > >>
> > >> Jun
> > >>
> > >> On Tue, Jun 9, 2020 at 3:37 PM Colin McCabe 
> wrote:
> > >>
> > >> > On Tue, Jun 9, 2020, at 05:06, David Jacot wrote:
> > >> > > Hi Colin,
> > >> > >
> > >> > > Thank you for your feedback.
> > >> > >
> > >> > > Jun has summarized the situation pretty well. Thanks Jun! I would
> > >> like to
> > >> > > complement it with the following points:
> > >> > >
> > >> > > 1. Indeed, when the quota is exceeded, the broker will reject the
> > >> topic
> > >> > > creations, partition creations and topics deletions that are
> exceeding
> > >> > > with the new QUOTA_VIOLATED error. The ThrottleTimeMs field will
> > >> > > be populated accordingly to let the client know how long it must
> wait.
> > >> > >
> > >> > > 2. I do agree that we actually want a mechanism to apply back
> pressure
> > >> > > to the clients. The KIP basically proposes a mechanism to control
> and
> > >> to
> > >> > > limit the rate of operations before entering the controller. I
> think
> > >> that
> > >> > > it is similar to your thinking but is enforced based on a defined
> > >> > > instead of relying on the number of pending items in the
> controller.
> > 

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-06-18 Thread David Jacot
+1 (non-binding)

Thanks for the KIP!

On Thu, Jun 18, 2020 at 12:00 AM Jose Garcia Sancio 
wrote:

> +1.
>
> Thanks for the KIP and looking forward to the improvement implementation.
>
> On Wed, Jun 17, 2020 at 2:24 PM Guozhang Wang  wrote:
> >
> > Thanks for the KIP Boyang, +1 from me.
> >
> >
> > Guozhang
> >
> > On Wed, Jun 17, 2020 at 1:40 PM Colin McCabe  wrote:
> >
> > > Thanks, Boyang!  +1 (binding)
> > >
> > > best,
> > > Colin
> > >
> > > On Mon, Jun 15, 2020, at 12:59, Boyang Chen wrote:
> > > > Thanks for more feedback Colin! I have addressed them in the KIP.
> > > >
> > > > Boyang
> > > >
> > > > On Mon, Jun 15, 2020 at 11:29 AM Colin McCabe 
> > > wrote:
> > > >
> > > > > On Fri, Jun 12, 2020, at 15:30, Boyang Chen wrote:
> > > > > > Thanks Colin for the suggestions!
> > > > > >
> > > > > > On Fri, Jun 12, 2020 at 2:40 PM Colin McCabe  >
> > > wrote:
> > > > > >
> > > > > > > Hi Boyang,
> > > > > > >
> > > > > > > Thanks for the KIP!  I think it's getting close.
> > > > > > >
> > > > > > >  > For older requests that need redirection, forwarding
> > > > > > >  > broker will just use its own authorizer to verify the
> > > principals.
> > > > > When
> > > > > > > the
> > > > > > >  > request looks good, it will just forward the request with
> its
> > > own
> > > > > > >  > credentials, no second validation needed
> > > > > > >
> > > > > > > Just to be clear, the controller will still validate the
> request,
> > > > > right?
> > > > > > > But at that point the principal will be the broker principal.
> It
> > > > > would be
> > > > > > > good to note that here.
> > > > > > >
> > > > > > > Sounds good, cleared in the KIP.
> > > > > >
> > > > > > > Internal CreateTopicsRequest Routing
> > > > > > >
> > > > > > > The forwarding broker is sending the request as the latest
> version,
> > > > > > > right?  It would be good to add a note of this.  This also
> prevents
> > > > > routing
> > > > > > > loops since the latest version is not forwardable (another good
> > > thing
> > > > > to
> > > > > > > add, I think...)
> > > > > > >
> > > > > > We are not bumping the CreateTopic RPC here, so it should be the
> > > latest
> > > > > > by default.
> > > > > >
> > > > >
> > > > > Sorry, CreateTopics was a bad example here, since it already must
> be
> > > sent
> > > > > to the controller.  Oops.
> > > > >
> > > > > >
> > > > > > And just to be clear, we are not "forwarding" but actually
> > > > > > sending a CreateTopicRequest from the receiving broker to the
> > > controller
> > > > > > broker.
> > > > > >
> > > > >
> > > > > Right.  I think we agree on this point.  But we do need a term to
> > > describe
> > > > > the broker which initially receives the user request and resends
> it to
> > > the
> > > > > controller.  Resending broker?
> > > > >
> > > > > And I do think it's important to note that the request we send to
> the
> > > > > controller can't be itself resent.
> > > > >
> > > > > >
> > > > > >  > As we discussed in the request routing section, to work with
> an
> > > older
> > > > > > >  > client, the first contacted broker need to act as a proxy to
> > > > > redirect
> > > > > > > the
> > > > > > >  > write request to the controller. To support the proxy of
> > > requests,
> > > > > we
> > > > > > > need
> > > > > > >  > to build a channel for brokers to talk directly to the
> > > controller.
> > > > > This
> > > > > > >  > part of the design is internal change only and won’t block
> the
> > > KIP
> > > > > > >  > progress.
> > > > > > >
> > > > > > > I think it's good to note that we eventually want a separate
> > > controller
> > > > > > > endpoint in KIP-500.  However, we don't need it to implement
> > > KIP-590,
> > > > > > > right?  The other brokers could forward to the existing
> internal
> > > > > endpoint
> > > > > > > for the controller.  So maybe it's best to discuss the separate
> > > > > endpoint in
> > > > > > > "future work" rather than here.
> > > > > > >
> > > > > > > I moved the new endpoint part towards the future work and
> > > addressed the
> > > > > > > usage of controller internal endpoint for routing requests.
> > > > > >
> > > > >
> > > > > Thanks.
> > > > >
> > > > > >
> > > > > > > > === Start Old Proposal  ===
> > > > > > >
> > > > > > > I'm glad the old proposal shows up here, but I think this is
> too
> > > much
> > > > > > > detail.  It would be better to just have a one or two paragraph
> > > > > summary of
> > > > > > > the main points.  As it is, the old proposal takes up 40% of
> the
> > > doc
> > > > > which
> > > > > > > is pretty confusing for someone reading through.  Let's also
> not
> > > forget
> > > > > > > that someone can just read the old version by using the "page
> > > history"
> > > > > > > function on the wiki.  So there's no need to keep that all
> here.
> > > > > > >
> > > > > > > Make sense, removed.
> > > > > >
> > > > >
> > > > > Thanks again.
> > > > >
> > > > > >
> > > > > >{ "name": "PrincipalName", "type": "stri

Re: [VOTE] KIP-597: MirrorMaker2 internal topics Formatters

2020-06-18 Thread David Jacot
Hi Mickael,

Thanks for your answer. Understood.

+1 (non-binding)

Best,
David

On Thu, Jun 18, 2020 at 10:15 AM Mickael Maison 
wrote:

> Hi David,
>
> Thanks again for your feedback!
>
> 2) I think it makes sense to use Configurable to make this interface
> consistent with all the other interfaces Kafka exposes. Yes it's
> unfortunate it introduces a deprecated method in the new interface but
> I don't think it's a big deal. It allows users to keep their existing
> code and just recompile it. They'll see deprecation warnings and will
> have time to move to the new interface. We will also remove the
> deprecated parts, including that method, in the next major release.
>
> On Thu, Jun 11, 2020 at 9:07 PM David Jacot  wrote:
> >
> > Hi Mickael,
> >
> > Thanks for the updated KIP.
> >
> > 1) I don’t feel too strong about this. I understand your point of view.
> >
> > 2) I just looked at the updated interface and I find a little weird to
> > already have a deprecated method in a new interface, don’t you? If we
> > believe that using Configurable is a good thing, it may be better to
> revise
> > our position regarding 1) and get rid of that deprecated init method.
> > Otherwise, it may be better to not implement Configurable after all. I
> > leave this up to you.
> >
> > 3) Thanks for the clarification.
> >
> > Best,
> > David
> >
> > Le jeu. 11 juin 2020 à 16:34, Mickael Maison 
> a
> > écrit :
> >
> > > Jun,
> > > Yes I'm aware this is allowed but I feel like it's a bit cheating =)
> > > Anyway it's a relatively minor KIP so let's do it, +1 binding
> > >
> > > David,
> > > 1) I understand compatibility is important but I feel like supporting
> > > both the old and new interface is excessive. The console tool does not
> > > mention the interface and only says a class can be passed so it's not
> > > really exposed.
> > > 2) I think it's a good idea, I'll update the KIP
> > > 3) Yes this is the plan and what I did in the PR. I can see it's not
> > > explicitly said in the KIP, I'll fix that
> > >
> > > Thanks
> > >
> > > On Wed, Jun 10, 2020 at 8:24 PM David Jacot 
> wrote:
> > > >
> > > > Hi Mickael,
> > > >
> > > > Thanks for the KIP. That looks very useful.
> > > >
> > > > I have few small comments/suggestions:
> > > >
> > > > 1. I was about the make a similar suggestion than Konstantine did
> > > regarding
> > > > requiring to recompile old formatters. While the formatters are not
> > > > directly part of the public API, I think that we can argue that, as
> they
> > > > are accepted by the console consumer, they are somehow part of it.
> It is
> > > a
> > > > bit a gray zone. With this in mind, I lean towards supporting both
> > > > interfaces by instantiating one or the other instead of making the
> old
> > > one
> > > > implements the new one. It is nicer for our users and require a
> similar
> > > > effort to implement overall.
> > > >
> > > > 2. Should the interface implement Configurable and Closable with
> default
> > > > empty implementations? That would make it similar to our other
> interfaces
> > > > but I am not entirely sure that works with the properties.
> > > >
> > > > 3. Should we directly move the existing Formatters to using the new
> > > > interface?
> > > >
> > > > Regards,
> > > > David
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > Le mer. 10 juin 2020 à 19:13, Maulin Vasavada <
> maulin.vasav...@gmail.com>
> > > a
> > > > écrit :
> > > >
> > > > > +1 (non-binding)
> > > > >
> > > > > On Wed, Jun 10, 2020 at 9:47 AM Mickael Maison <
> > > mickael.mai...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Bumping this thread. Let me know if you have any questions or
> > > feedback.
> > > > > >
> > > > > > So far, we have 2 binding and 5 non-binding votes.
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > > > On Tue, May 19, 2020 at 10:56 AM Manikumar <
> > > manikumar.re...@gmail.com>
> &

Re: [DISCUSS] KIP-431: Support of printing additional ConsumerRecord fields in DefaultMessageFormatter

2020-06-18 Thread David Jacot
Hi Badai,

Thanks for resuming this. I have few small comments:

1. It seems that `print.partition` is already implemented. Do you confirm?

2. Will `null.literal` be only used when the value of the message
is NULL or for any fields? Also, it seems that we print out "null"
today when the key or the value is empty. Shall we use "null" as
a default instead of ""?

3. Could we add a small example of the output in the KIP?

4. When there are no headers, are we going to print something
to indicate it to the user? For instance, we print out NO_TIMESTAMP
where there is no timestamp.

Best,
David

On Wed, Jun 17, 2020 at 4:53 PM Badai Aqrandista  wrote:

> Hi all,
>
> I have contacted Mateusz separately and he is ok for me to take over
> KIP-431:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-431%3A+Support+of+printing+additional+ConsumerRecord+fields+in+DefaultMessageFormatter
>
> I have updated it a bit. Can anyone give a quick look at it again and
> give me some feedback?
>
> This feature will be very helpful for people supporting Kafka in
> operations.
>
> If it is ready for a vote, please let me know.
>
> Thanks
> Badai
>
> On Sat, Jun 13, 2020 at 10:59 PM Badai Aqrandista 
> wrote:
> >
> > Mateusz
> >
> > This KIP would be very useful for debugging. But the last discussion
> > is in Feb 2019.
> >
> > Are you ok if I take over this KIP?
> >
> > --
> > Thanks,
> > Badai
>
>
>
> --
> Thanks,
> Badai
>


Re: [DISCUSS] KIP-431: Support of printing additional ConsumerRecord fields in DefaultMessageFormatter

2020-06-19 Thread David Jacot
Hi Badai,

Thanks for your reply.

2. Yes, that makes sense.

Best,
David

On Thu, Jun 18, 2020 at 2:08 PM Badai Aqrandista  wrote:

> David
>
> Thank you for replying
>
> 1. It seems that `print.partition` is already implemented. Do you confirm?
> BADAI: Yes, you are correct. I have removed it from the KIP.
>
> 2. Will `null.literal` be only used when the value of the message
> is NULL or for any fields? Also, it seems that we print out "null"
> today when the key or the value is empty. Shall we use "null" as
> a default instead of ""?
> BADAI: For any fields. Do you think this is useful?
>
> 3. Could we add a small example of the output in the KIP?
> BADAI: Yes, I have updated the KIP to add a couple of example.
>
> 4. When there are no headers, are we going to print something
> to indicate it to the user? For instance, we print out NO_TIMESTAMP
> where there is no timestamp.
> BADAI: Yes, good idea. I have updated the KIP to print NO_HEADERS.
>
> Thanks
> Badai
>
>
> On Thu, Jun 18, 2020 at 7:25 PM David Jacot  wrote:
> >
> > Hi Badai,
> >
> > Thanks for resuming this. I have few small comments:
> >
> > 1. It seems that `print.partition` is already implemented. Do you
> confirm?
> >
> > 2. Will `null.literal` be only used when the value of the message
> > is NULL or for any fields? Also, it seems that we print out "null"
> > today when the key or the value is empty. Shall we use "null" as
> > a default instead of ""?
> >
> > 3. Could we add a small example of the output in the KIP?
> >
> > 4. When there are no headers, are we going to print something
> > to indicate it to the user? For instance, we print out NO_TIMESTAMP
> > where there is no timestamp.
> >
> > Best,
> > David
> >
> > On Wed, Jun 17, 2020 at 4:53 PM Badai Aqrandista 
> wrote:
> >
> > > Hi all,
> > >
> > > I have contacted Mateusz separately and he is ok for me to take over
> > > KIP-431:
> > >
> > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-431%3A+Support+of+printing+additional+ConsumerRecord+fields+in+DefaultMessageFormatter
> > >
> > > I have updated it a bit. Can anyone give a quick look at it again and
> > > give me some feedback?
> > >
> > > This feature will be very helpful for people supporting Kafka in
> > > operations.
> > >
> > > If it is ready for a vote, please let me know.
> > >
> > > Thanks
> > > Badai
> > >
> > > On Sat, Jun 13, 2020 at 10:59 PM Badai Aqrandista 
> > > wrote:
> > > >
> > > > Mateusz
> > > >
> > > > This KIP would be very useful for debugging. But the last discussion
> > > > is in Feb 2019.
> > > >
> > > > Are you ok if I take over this KIP?
> > > >
> > > > --
> > > > Thanks,
> > > > Badai
> > >
> > >
> > >
> > > --
> > > Thanks,
> > > Badai
> > >
>
>
>
> --
> Thanks,
> Badai
>


Re: [VOTE] KIP-627: Expose Trogdor-specific JMX Metrics for Tasks and Agents

2020-06-26 Thread David Jacot
+1 (non-binding)

Thanks for the KIP!

Best,
David

On Fri, Jun 26, 2020 at 11:11 AM Manikumar 
wrote:

> +1 (binding)
>
> Thanks for the KIP.
>
> Thans,
> Manikumar
>
> On Fri, Jun 26, 2020 at 11:46 AM Stanislav Kozlovski <
> stanis...@confluent.io>
> wrote:
>
> > +1 (non-binding).
> >
> > Thanks for the work! I am also happy to see Trogdor being improved
> >
> > Best,
> > Stanislav
> >
> > On Fri, Jun 26, 2020 at 5:34 AM Colin McCabe  wrote:
> >
> > > +1 (binding).
> > >
> > > Thanks, Sam.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Thu, Jun 25, 2020, at 18:05, Gwen Shapira wrote:
> > > > +1 (binding)
> > > >
> > > > Thank you, Sam. It is great to see Trogdor getting the care it
> > deserves.
> > > >
> > > > On Mon, Jun 22, 2020, 1:46 PM Sam Pal  wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I would like to start a vote for KIP-627, which adds metrics about
> > > active
> > > > > agents and the number of created, running, and done tasks in a
> > Trogdor
> > > > > cluster:
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-627%3A+Expose+Trogdor-specific+JMX+Metrics+for+Tasks+and+Agents
> > > > >
> > > > > Looking forward to hearing from you all!
> > > > >
> > > > > Best,
> > > > > Sam
> > > > >
> > > > >
> > > >
> > >
> >
> >
> > --
> > Best,
> > Stanislav
> >
>


Re: [VOTE] KIP-629: Use racially neutral terms in our codebase

2020-06-26 Thread David Jacot
+1 (non-binding). Thanks, Xavier!

On Fri, Jun 26, 2020 at 10:59 AM Levani Kokhreidze 
wrote:

> +1 (non-binding)
>
> Thank you for this initiative.
>
> Levani
>
> > On Jun 26, 2020, at 11:53 AM, Mickael Maison 
> wrote:
> >
> > +1 (binding)
> > Thanks for the KIP!
> >
> > On Fri, Jun 26, 2020 at 9:51 AM Jorge Esteban Quilcate Otoya
> >  wrote:
> >>
> >> +1 (non-binding)
> >> Thank you Xavier!
> >>
> >> On Fri, Jun 26, 2020 at 8:38 AM Bruno Cadonna 
> wrote:
> >>
> >>> +1 (non-binding)
> >>>
> >>> On Fri, Jun 26, 2020 at 3:41 AM Jay Kreps  wrote:
> 
>  +1
> 
>  On Thu, Jun 25, 2020 at 6:39 PM Bill Bejeck 
> wrote:
> 
> > Thanks for this KIP Xavier.
> >
> > +1(binding)
> >
> > -Bill
> >
> > On Thu, Jun 25, 2020 at 9:04 PM Gwen Shapira 
> >>> wrote:
> >
> >> +1 (binding)
> >>
> >> Thank you Xavier!
> >>
> >> On Thu, Jun 25, 2020, 3:44 PM Xavier Léauté 
> wrote:
> >>
> >>> Hi Everyone,
> >>>
> >>> I would like to initiate the voting process for KIP-629.
> >>>
> >>>
> >>
> >
> >>>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-629%3A+Use+racially+neutral+terms+in+our+codebase
> >>>
> >>> Thank you,
> >>> Xavier
> >>>
> >>
> >
> >>>
>
>


Re: [ANNOUNCE] New committer: Boyang Chen

2020-06-26 Thread David Jacot
Congrats, Boyang!

On Thu, Jun 25, 2020 at 1:57 PM Viktor Somogyi-Vass 
wrote:

> Congrats :)
>
> On Thu, Jun 25, 2020 at 12:28 AM Liquan Pei  wrote:
>
> > Congrats!
> >
> > On Wed, Jun 24, 2020 at 9:42 AM Raymond Ng  wrote:
> >
> > > Congrats Boyang! Look forward to more awesome contributions from you in
> > the
> > > future.
> > >
> > > Regards,
> > > Ray
> > >
> > > On Wed, Jun 24, 2020 at 6:07 AM Ismael Juma  wrote:
> > >
> > > > Congratulations Boyang!
> > > >
> > > > Ismael
> > > >
> > > > On Mon, Jun 22, 2020 at 4:26 PM Guozhang Wang 
> > > wrote:
> > > >
> > > > > The PMC for Apache Kafka has invited Boyang Chen as a committer and
> > we
> > > > are
> > > > > pleased to announce that he has accepted!
> > > > >
> > > > > Boyang has been active in the Kafka community more than two years
> > ago.
> > > > > Since then he has presented his experience operating with Kafka
> > Streams
> > > > at
> > > > > Pinterest as well as several feature development including
> rebalance
> > > > > improvements (KIP-345) and exactly-once scalability improvements
> > > > (KIP-447)
> > > > > in various Kafka Summit and Kafka Meetups. More recently he's also
> > been
> > > > > participating in Kafka broker development including post-Zookeeper
> > > > > controller design (KIP-500). Besides all the code contributions,
> > Boyang
> > > > has
> > > > > also helped reviewing even more PRs and KIPs than his own.
> > > > >
> > > > > Thanks for all the contributions Boyang! And look forward to more
> > > > > collaborations with you on Apache Kafka.
> > > > >
> > > > >
> > > > > -- Guozhang, on behalf of the Apache Kafka PMC
> > > > >
> > > >
> > >
> > --
> > Liquan Pei
> > Software Engineer, Confluent Inc
> >
>


Re: Re:Re: [ANNOUNCE] New committer: Xi Hu

2020-06-26 Thread David Jacot
Congrats!

On Thu, Jun 25, 2020 at 4:08 PM Hu Xi  wrote:

> Thank you, everyone. It is my great honor to be a part of the community.
> Will make a greater contribution in the coming days.
>
> 
> 发件人: Roc Marshal 
> 发送时间: 2020年6月25日 10:20
> 收件人: us...@kafka.apache.org 
> 主题: Re:Re: [ANNOUNCE] New committer: Xi Hu
>
> Congratulations ! Xi Hu.
>
>
> Best,
> Roc Marshal.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> At 2020-06-25 01:30:33, "Boyang Chen"  wrote:
> >Congratulations Xi! Well deserved.
> >
> >On Wed, Jun 24, 2020 at 10:10 AM AJ Chen  wrote:
> >
> >> Congratulations, Xi.
> >> -aj
> >>
> >>
> >>
> >> On Wed, Jun 24, 2020 at 9:27 AM Guozhang Wang 
> wrote:
> >>
> >> > The PMC for Apache Kafka has invited Xi Hu as a committer and we are
> >> > pleased to announce that he has accepted!
> >> >
> >> > Xi Hu has been actively contributing to Kafka since 2016, and is well
> >> > recognized especially for his non-code contributions: he maintains a
> tech
> >> > blog post evangelizing Kafka in the Chinese speaking community (
> >> > https://www.cnblogs.com/huxi2b/), and is one of the most active
> >> answering
> >> > member in Zhihu (Chinese Reddit / StackOverflow) Kafka topic. He has
> >> > presented in Kafka meetup events in the past and authored a
> >> > book deep-diving on Kafka architecture design and operations as well (
> >> > https://www.amazon.cn/dp/B07JH9G2FL). Code wise, he has contributed
> 75
> >> > patches so far.
> >> >
> >> >
> >> > Thanks for all the contributions Xi. Congratulations!
> >> >
> >> > -- Guozhang, on behalf of the Apache Kafka PMC
> >> >
> >>
>


Re: [DISCUSS] Apache Kafka 2.6.0 release

2020-06-29 Thread David Jacot
Hi Randall,

We have discovered an annoying issue that we introduced in 2.5:

Describing topics with the command line tool fails if the user does not
have the
privileges to access the ListPartitionReassignments API. I believe that
this is the
case for most non-admin users.

I propose to include the fix in 2.6. The fix is trivial so low risk. What
do you think?

JIRA: https://issues.apache.org/jira/browse/KAFKA-10212
PR: https://github.com/apache/kafka/pull/8947

Best,
David

On Sat, Jun 27, 2020 at 4:39 AM John Roesler  wrote:

> Hi Randall,
>
> I neglected to notify this thread when I merged the fix for
> https://issues.apache.org/jira/browse/KAFKA-10185
> on June 19th. I'm sorry about that oversight. It is marked with
> a fix version of 2.6.0.
>
> On a side node, I have a fix for KAFKA-10173, which I'm merging
> and backporting right now.
>
> Thanks for managing the release,
> -John
>
> On Thu, Jun 25, 2020, at 10:23, Randall Hauch wrote:
> > Thanks for the update, folks!
> >
> > Based upon Jira [1], we currently have 4 issues that are considered
> > blockers for the 2.6.0 release and production of RCs:
> >
> >- https://issues.apache.org/jira/browse/KAFKA-10134 - High CPU issue
> >during rebalance in Kafka consumer after upgrading to 2.5 (unassigned)
> >- https://issues.apache.org/jira/browse/KAFKA-10143 - Can no longer
> >change replication throttle with reassignment tool (Jason G)
> >- https://issues.apache.org/jira/browse/KAFKA-10166 - Excessive
> >TaskCorruptedException seen in testing (Sophie, Bruno)
> >- https://issues.apache.org/jira/browse/KAFKA-10173
> >- BufferUnderflowException during Kafka Streams Upgrade (John R)
> >
> > and one critical issue that may be a regression that at this time will
> not
> > block production of RCs:
> >
> >- https://issues.apache.org/jira/browse/KAFKA-10017 - Flaky Test
> >EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta
> (Matthias)
> >
> > and one build/release issue we'd like to fix if possible but will not
> block
> > RCs or the release:
> >
> >- https://issues.apache.org/jira/browse/KAFKA-9381
> >- kafka-streams-scala: Javadocs + Scaladocs not published on maven
> central
> >(me)
> >
> > I'm working with the assignees and reporters of these issues (via
> comments
> > on the issues) to identify an ETA and to track progress. Anyone is
> welcome
> > to chime in on those issues.
> >
> > At this time, no other changes (other than PRs that only fix/improve
> tests)
> > should be merged to the `2.6` branch. If you think you've identified a
> new
> > blocker issue or believe another existing issue should be treated as a
> > blocker for 2.6.0, please mark the issue's `fix version` as `2.6.0` _and_
> > respond to this thread with details, and I will work with you to
> determine
> > whether it is indeed a blocker.
> >
> > As always, let me know here if you have any questions/concerns.
> >
> > Best regards,
> >
> > Randall
> >
> > [1] https://issues.apache.org/jira/projects/KAFKA/versions/12346918
> >
> > On Thu, Jun 25, 2020 at 8:27 AM Mario Molina  wrote:
> >
> > > Hi Randal,
> > >
> > > Ticket https://issues.apache.org/jira/browse/KAFKA-9018 is not a
> blocker
> > > so
> > > it can be moved to the 2.7.0 version.
> > >
> > > Mario
> > >
> > > On Wed, 24 Jun 2020 at 20:22, Boyang Chen 
> > > wrote:
> > >
> > > > Hey Randal,
> > > >
> > > > There was another spotted blocker:
> > > > https://issues.apache.org/jira/browse/KAFKA-10173
> > > > As of current, John is working on a fix.
> > > >
> > > > Boyang
> > > >
> > > > On Wed, Jun 24, 2020 at 4:08 PM Sophie Blee-Goldman <
> sop...@confluent.io
> > > >
> > > > wrote:
> > > >
> > > > > Hey all,
> > > > >
> > > > > Just a heads up that we discovered a new blocker. The fix is pretty
> > > > > straightforward
> > > > > and there's already a PR for it so it should be resolved quickly.
> > > > >
> > > > > Here's the ticket:
> https://issues.apache.org/jira/browse/KAFKA-10198
> > > > >
> > > > > On Sat, May 30, 2020 at 12:52 PM Randall Hauch 
> > > wrote:
> > > > >
> > > > > > Hi, Kowshik,
> > > > > >
> > > > > > Thanks for the update on KIP-584. This is listed on the
> "Postponed"
> > > > > section
> > > > > > of the AK 2.6.0 release plan (
> > > > > >
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=152113430
> > > > > > ).
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Randall
> > > > > >
> > > > > > On Fri, May 29, 2020 at 4:51 PM Kowshik Prakasam <
> > > > kpraka...@confluent.io
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Randall,
> > > > > > >
> > > > > > > We have to remove KIP-584 from the release plan, as this item
> will
> > > > not
> > > > > be
> > > > > > > completed for 2.6 release (although KIP is accepted). We plan
> to
> > > > > include
> > > > > > it
> > > > > > > in a next release.
> > > > > > >
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Kowshik
> > > > > > >
> > > > >

Re: [DISCUSS] KIP-632: Add DirectoryConfigProvider

2020-07-01 Thread David Jacot
Hi Tom,

I think that it is a useful addition. The KIP looks good to me.

Thanks,
David

Le mer. 1 juil. 2020 à 18:58, Manikumar  a
écrit :

> Hi,
>
> Thanks for the KIP.
> KIP looks good to me and will be a useful addition.
>
> Thanks,
> Manikumar
>
> On Mon, Jun 29, 2020 at 4:05 PM Tom Bentley  wrote:
>
> > Hi all,
> >
> > I would like to start discussion on a small KIP to make using the config
> > provider mechanism more ergonomic on Kubernetes:
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-632%3A+Add+DirectoryConfigProvider
> >
> > If you have a few minutes to have a look I'd be grateful for any
> feedback.
> >
> > Many thanks,
> >
> > Tom
> >
>


Re: [VOTE] KIP-632: Add DirectoryConfigProvider

2020-07-07 Thread David Jacot
+1 (non-binding). Thanks for the KIP!

On Tue, Jul 7, 2020 at 12:54 PM Tom Bentley  wrote:

> Hi,
>
> I'd like to start a vote on KIP-632, which is about making the config
> provider mechanism more ergonomic on Kubernetes:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-632%3A+Add+DirectoryConfigProvider
>
> Please take a look if you have time.
>
> Many thanks,
>
> Tom
>


Re: [VOTE] KIP-621: Deprecate and replace DescribeLogDirsResult.all() and .values()

2020-07-09 Thread David Jacot
I couldn't think of a better alternative either. Thanks for the KIP!

+1 (non-binding)

On Wed, Jul 8, 2020 at 12:42 PM Manikumar  wrote:

> +1 (binding)
>
> Thanks for the KIP.
>
> On Tue, Jul 7, 2020 at 8:19 PM Colin McCabe  wrote:
>
> > Thanks, Tom.
> >
> > I tried to think of a better way to do this, but I think you're right
> that
> > we probably just need different methods.
> >
> > +1 (binding).
> >
> > best,
> > Colin
> >
> > On Mon, Jul 6, 2020, at 01:14, Tom Bentley wrote:
> > > Hi,
> > >
> > > I'd like to start a vote on KIP-621 which is about deprecating methods
> in
> > > DescribeLogDirsResult which leak internal classes, replacing them with
> > > public API classes.
> > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158862109
> > >
> > > Thanks,
> > >
> > > Tom
> > >
> >
>


Re: [VOTE] KIP-431: Support of printing additional ConsumerRecord fields in DefaultMessageFormatter

2020-07-09 Thread David Jacot
Hi Badai,

Thanks for the KIP. I think that it is a nice improvement so I am +1
(non-binding).

Long term, I wonder if we could adopt a formatting system similar to
kafkacat. It
would reduce the number of properties that one has to set and also allow
more
powerful formatting. That could be done as a new formatter for instance.

Example:
kafkacat -b mybroker -t syslog -f 'Topic %t[%p], offset: %o, key: %k,
payload: %S bytes: %s\n'

Best,
David

On Wed, Jul 8, 2020 at 12:30 PM Manikumar  wrote:

> +1 (binding)
>
> Thanks for the KIP.
>
> Thanks,
> Manikumar
>
>
>
> On Wed, Jul 8, 2020 at 11:47 AM Matthias J. Sax  wrote:
>
> > +1 (binding)
> >
> > -Matthias
> >
> > On 7/7/20 7:16 PM, John Roesler wrote:
> > > Hi Badai,
> > >
> > > Thanks for picking this up. I've reviewed the KIP document and
> > > the threads you linked. I think we may want to make more
> > > improvements in the future to the printing of headers in particular,
> > > but this KIP seems like a clear benefit already. I think we can
> > > take it incrementally.
> > >
> > > I'm +1 (binding)
> > >
> > > Thanks,
> > > -John
> > >
> > > On Tue, Jul 7, 2020, at 09:57, Badai Aqrandista wrote:
> > >> Hi all
> > >>
> > >> After resurrecting the discussion thread [1] for KIP-431 and have not
> > >> received any further feedback for 2 weeks, I would like to resurrect
> > >> the voting thread [2] for KIP-431.
> > >>
> > >> I have updated KIP-431 wiki page
> > >> (
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-431%3A+Support+of+printing+additional+ConsumerRecord+fields+in+DefaultMessageFormatter
> > )
> > >> to address Ismael's comment on that thread [3].
> > >>
> > >> Does anyone else have other comments or objections about this KIP?
> > >>
> > >> [1]
> > >>
> >
> https://lists.apache.org/thread.html/raabf3268ed05931b8a048fce0d6cdf6a326aee4b0d89713d6e6998d6%40%3Cdev.kafka.apache.org%3E
> > >>
> > >> [2]
> > >>
> >
> https://lists.apache.org/thread.html/41fff34873184625370f9e76b8d9257f7a9e7892ab616afe64b4f67c%40%3Cdev.kafka.apache.org%3E
> > >>
> > >> [3]
> > >>
> >
> https://lists.apache.org/thread.html/99e9cbaad4a0a49b96db104de450c9f488d4b2b03a09b991bcbadbc7%40%3Cdev.kafka.apache.org%3E
> > >>
> > >> --
> > >> Thanks,
> > >> Badai
> > >>
> >
> >
>


Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-28 Thread David Jacot
Hi Boyang,

Thanks for the update.

1./2. In KIP-599 (accepted and already in trunk), we throttle the
CreateTopicsRequest,
CreatePartitionsRequest, and DeleteTopicsRequests by muting the channel used
by the Admin client and setting the throttleTimeMs in the response. The
change that
you propose breaks this. If we distribute these requests to all brokers,
muting channels
does not make sense anymore.

Have we considered continuing sending controller requests to the broker
that hosts the
controller? We could continue to send requests to the broker listener and
redirect them
to the controller internally. This would still keep the controller
isolated. Another advantage
of doing so is that the forwarding on the same machine does not require to
go over the
network.

Could you elaborate a bit more on the motivation and the reasoning behind
this change?
Is there a requirement or a strong advantage that I have missed?

Best,
David

On Tue, Jul 28, 2020 at 6:48 AM Boyang Chen 
wrote:

> Hey there,
>
> I'm re-opening this thread because after some initial implementations
> started, we spotted some gaps in the approved KIP as well as some
> inconsistencies with KIP-631 controller. There are a couple of addendums to
> the existing KIP, specifically:
>
> 1. As the controller is foreseen to be only accessible to the brokers, the
> new admin client would not have direct access to the controller. It is
> guaranteed on the MetadataResponse level which no longer provides
> `ControllerId` to client side requests.
>
> 2. The broker would forward any direct ZK path mutation requests, including
> topic creation/deletion, reassignment, etc since we deprecate the direct
> controller access on the client side. No more protocol version bump is
> necessary for the configuration requests.
>
> 3. To make sure forwarding requests pass the authorization, broker
> principal CLUSTER_ACTION would be allowed to be used as an alternative
> authentication method for a variety of principal operations, including
> ALTER, ALTER_CONFIG, DELETE, etc. It is because the forwarding request
> needs to use the proxy broker's own principal, which is currently not
> supported to be used for many configuration change authentication listed
> above. The full list could be found in the KIP.
>
> 4. Add a new BROKER_AUTHORIZATION_FAILURE error code to indicate any
> internal security configuration failure, when the forwarded request failed
> authentication on the controller side.
>
> Let me know what you think. With such a major refinement of the KIP, I'm
> open for re-vote after discussions converge.
>
> Boyang
>
> On Wed, Jul 1, 2020 at 2:17 PM Boyang Chen 
> wrote:
>
> > Hey folks,
> >
> > I have also synced on the KIP-578 which was doing the partition limit, to
> > make sure the partition limit error code would be properly propagated
> once
> > it is done on top of KIP-590. Let me know if you have further questions
> or
> > concerns.
> >
> > Boyang
> >
> > On Tue, Jun 23, 2020 at 5:08 PM Boyang Chen 
> > wrote:
> >
> >> Thanks for the clarification, Colin and Ismael. Personally I also feel
> >> Option A is better to prioritize fixing the gap. Just to be clear, the
> >> proposed solution would be:
> >>
> >> 1. Bump the Metadata RPC version to return POLICY_VIOLATION. In the
> >> application level, we should swap the error message with the actual
> failure
> >> reason such as "violation of topic creation policy when attempting to
> auto
> >> create internal topic through MetadataRequest."
> >>
> >> 2. For older Metadata RPC, return AUTHORIZATION_FAILED to fail fast.
> >>
> >> Will address our other discussed points as well in the KIP, let me know
> >> if you have further questions.
> >>
> >> Thanks,
> >> Boyang
> >>
> >> On Tue, Jun 23, 2020 at 10:41 AM Ismael Juma  wrote:
> >>
> >>> Option A is basically what I was thinking. But with a slight
> adjustment:
> >>>
> >>> New versions of MetadataResponse return POLICY_VIOLATION, old versions
> >>> return AUTHORIZATION_FAILED. The latter works correctly with old Java
> >>> clients (i.e. the client fails fast and propagates the error), I've
> >>> tested
> >>> it. Adjust new clients to treat POLICY_VIOLATION like
> >>> AUTHORIZATION_FAILED,
> >>> but propagate the custom error message.
> >>>
> >>> Ismael
> >>>
> >>> On Mon, Jun 22, 2020 at 11:00 PM Colin McCabe 
> >>> wrote:
> >>>
> >>> > > > > On Fri, Jun 19, 2020 at 3:18 PM Ismael Juma  >
> >>> > wrote:
> >>> > > > >
> >>> > > > > > Hi Colin,
> >>> > > > > >
> >>> > > > > > The KIP states in the Compatibility section (not Future
> work):
> >>> > > > > >
> >>> > > > > > "To support the proxy of requests, we need to build a channel
> >>> for
> >>> > > > > > brokers to talk directly to the controller. This part of the
> >>> design
> >>> > > > > > is internal change only and won’t block the KIP progress."
> >>> > > > > >
> >>> > > > > > I am clarifying that this is not internal only due to the
> >>> config.
> >>> > If we
> >>> > > > > > say that this KIP depends on an

Re: [VOTE] KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations

2020-07-28 Thread David Jacot
Hi all,

Just a quick update. We have made good progress regarding the implementation
of this KIP. The major parts are already in trunk modulo the new "rate"
implementation
which is still under development.

I would like to change the type of the `controller_mutations_rate` from a
Long to
a Double. I have made various experiments and the rate can be quite low,
especially
in clusters with a lot of tenants. Using a Long is quite limiting here to
fine tune small
rates. I hope that this change is fine for everyone.

Best,
David

On Mon, Jun 15, 2020 at 9:26 AM David Jacot  wrote:

> Hi all,
>
> The vote has passed with 5 binding votes (Gwen, Rajini, Mickael, Jun and
> Colin)
> and 2 non-binding votes (Tom, Anna).
>
> Thank you all for the fruitful discussion! I'd like to particularly thank
> Anna who has
> heavily contributed to the design of this KIP.
>
> Regards,
> David
>
> On Fri, Jun 12, 2020 at 10:08 PM Colin McCabe  wrote:
>
>> +1.  Thanks, David!
>>
>> best,
>> Colin
>>
>> On Thu, Jun 11, 2020, at 23:51, David Jacot wrote:
>> > Colin, Jun,
>> >
>> > Do the proposed error code and the updated KIP look good to you guys?
>> I’d
>> > like to wrap up and close the vote.
>> >
>> > Thanks,
>> > David
>> >
>> > Le mer. 10 juin 2020 à 14:50, David Jacot  a
>> écrit :
>> >
>> > > Hi Colin and Jun,
>> > >
>> > > I have no problem if we have to rewrite part of it when the new
>> controller
>> > > comes
>> > > out. I will be more than happy to help out.
>> > >
>> > > Regarding KIP-590, I think that we can cope with a principal as a
>> string
>> > > when the
>> > > time comes. The user entity name is defined with a string already.
>> > >
>> > > Regarding the name of the error, you have made a good point. I do
>> agree
>> > > that it
>> > > is important to differentiate the two cases. I propose the following
>> two
>> > > errors:
>> > > - THROTTLING_QUOTA_EXCEEDED - Throttling is slightly better than rate
>> as
>> > > we have quotas which are not rate (e.g. request quota). This one is
>> > > retryable
>> > > once the throttle time is passed.
>> > > - LIMIT_QUOTA_EXCEEDED - This one would indicate that the limit has
>> been
>> > > reached and is a final error.
>> > > We only need the former in this KIP. What do you think?
>> > >
>> > > Jun, I have added a few examples in the KIP. The new name works
>> exactly
>> > > like
>> > > the existing one once it is added to the accepted dynamic configs for
>> the
>> > > user
>> > > and the client entities. I have added a "Kafka Config Command"
>> chapter in
>> > > the
>> > > KIP. I will also open a Jira to not forget updating the AK
>> documentation
>> > > once
>> > > the KIP gets merged.
>> > >
>> > > Thanks,
>> > > David
>> > >
>> > > On Wed, Jun 10, 2020 at 3:03 AM Jun Rao  wrote:
>> > >
>> > >> Hi, Colin,
>> > >>
>> > >> Good point. Maybe sth like THROTTLING_QUOTA_VIOLATED will make this
>> clear.
>> > >>
>> > >> Hi, David,
>> > >>
>> > >> We added a new quota name in the KIP. You chose not to bump up the
>> version
>> > >> of DESCRIBE_CLIENT_QUOTAS and ALTER_CLIENT_QUOTAS, which seems ok
>> since
>> > >> the
>> > >> quota name is represented as a string. However, the new quota name
>> can be
>> > >> used in client tools for setting and listing the quota (
>> > >> https://kafka.apache.org/documentation/#quotas). Could you document
>> how
>> > >> the
>> > >> new name will be used in those tools?
>> > >>
>> > >> Thanks,
>> > >>
>> > >> Jun
>> > >>
>> > >> On Tue, Jun 9, 2020 at 3:37 PM Colin McCabe 
>> wrote:
>> > >>
>> > >> > On Tue, Jun 9, 2020, at 05:06, David Jacot wrote:
>> > >> > > Hi Colin,
>> > >> > >
>> > >> > > Thank you for your feedback.
>> > >> > >
>> > >> > > Jun has summarized the situation pretty well. Thanks Jun! I would
>> > >> like to
>> > >> > > complemen

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-29 Thread David Jacot
Hi, Colin, Boyang,

Colin, thanks for the clarification. Somehow, I thought that even if the
controller is ran independently, it
would still run the listeners of the broker and thus would be accessible by
redirecting on the loopback
interface. My mistake.

Boyang, I have few questions/comments regarding the updated KIP:

1. I think that it would be great if we could clarify how old admin clients
which are directly talking to the
controller will work with this KIP. I read between the lines that, as we
propose to provide a random
broker Id as the controller Id in the metadata response, they will use a
single node as a proxy. Is that
correct? This deserves to be called out more explicitly in the design
section instead of being hidden
in the protocol bump of the metadata RPC.

1.1 If I understand correctly, could we assume that old admin clients will
stick to the same "fake controller"
until they refresh their metadata? Refreshing the metadata usually happens
when NOT_CONTROLLER
is received but this won't happen anymore so they should change
infrequently.

2. For the new admin client, I suppose that we plan on using
LeastLoadedNodeProvider for the
requests that are using ControllerNodeProvider. We could perhaps mention it.

3. Pre KIP-500, will we have a way to distinguish if a request that is
received by the controller is
coming directly from a client or from a broker? You mention that the
listener can be used to do
this but as you pointed out, it is not mandatory. Do we have another
reliable method? I am asking
in the context of KIP-599 with the current controller, we may need to
throttle differently if the
request comes from a client or from a broker.

4. Could we add `InitialClientId` as well? This will be required for the
quota as we can apply them
by principal and/or clientId.

5. A small remark regarding the structure of the KIP. It is a bit weird
that requests that do not go
to the controller are mentioned in the Proposed Design section and the
requests that go to the
controller are mentioned in the Public Interfaces. When one read the
Proposed Design, it does not
get a full picture of the whole new routing proposal for old and new
clients. It would be great if we
could have a full overview in that section.

Overall the change makes sense to me. I will work on drafting an addendum
to KIP-599 to
alter the design to cope with these changes. At a first glance, that seems
doable if 1.1, 3
and 4 are OK.

Thanks,
David

On Wed, Jul 29, 2020 at 5:29 AM Boyang Chen 
wrote:

> Thanks for the feedback Colin!
>
> On Tue, Jul 28, 2020 at 2:11 PM Colin McCabe  wrote:
>
> > Hi Boyang,
> >
> > Thanks for updating this.  A few comments below:
> >
> > In the "Routing Request Security" section, there is a reference to "older
> > requests that need redirection."  But after these new revisions, both new
> > and old requests need redirection.  So we should rephrase this.
> >
> > > In addition, to avoid exposing this forwarding power to the admin
> > clients,
> > > the routing request shall be forwarded towards the controller broker
> > internal
> > > endpoint which should be only visible to other brokers inside the
> > cluster
> > > in the KIP-500 controller. Any admin configuration request with broker
> > > principal should not be going through the public endpoint and will be
> > > rejected for security purpose.
> >
> > We should also describe how this will work in the pre-KIP-500 case.  In
> > that case, CLUSTER_ACTION gets the extra permissions described here only
> > when the message comes in on the inter-broker listener.  We should state
> > that here.
> >
> > (I can see that you have this information later on in the "Security
> Access
> > Changes" section, but it would be good to have it here as well, to avoid
> > confusion.)
> >
> > > To be more strict of protecting controller information, the
> > "ControllerId"
> > > field in new MetadataResponse shall be set to -1 when the original
> > request
> > > comes from a non-broker client and it is already on v10. We shall use
> the
> > > request listener name to distinguish whether a given request is
> > inter-broker,
> > > or from the client.
> >
> > I'm not sure why we would need to distinguish between broker clients and
> > non-broker clients.  Brokers don't generally send MetadataRequests to
> other
> > brokers, do they?  Brokers learn about metadata from
> UpdateMetadataRequest
> > and LeaderAndIsrRequest, not by sending MetadataRequests to other
> brokers.
> >
> > We do have one use case where the MetadataRequest gets sent between the
> brokers, which is the InterBrokerSendThread. Currently we don't rely on it
> to get the controller id, so I guess your suggestion should be good to
> enforce. We could use some meta comment on the NetworkClient that it should
> not be used to get the controller location.
>
> Probably what we want here is: v0-v9: return a random broker in the cluster
> > as the controller ID.  v10: no controllerID present in the
> > MetadataResponse. 

Re: [VOTE] KIP-590: Redirect Zookeeper Mutation Protocols to The Controller

2020-07-30 Thread David Jacot
Hi Boyang,

Thanks for your answers.

> The point for using the listener name is more of a security purpose, to
> detect any forged request to our best effort.
> For throttling I think we could just check the request header for
> *InitialClientId* existence, to distinguish whether to apply
> throttling strategy as forwarded request or direct request.

Reading "security" and "best effort" in the same sentence makes me a
little nervous :).

The identification issue is also valid for quota as we don't want one to be
able to bypass the quota by forging a request as well, isn't it? Otherwise,
anyone could just set the InitialPrincipal to bypass it. I think that we
should
only use InitialPrincipal and/or InitialClientId when we know that they come
from another broker. Based on what I read in the KIP, it looks like we could
only use them when the principal has CLUSTER_ACTION privilege. Do I
understand it correctly?

I have made another pass on the whole KIP, I have few nits:

- The sentence "Take AlterConfig as an example to understand the changes
we are making." does not make much sense anymore in the beginning of the
"Proposed Changes" chapter.

- When you say "Existing RPCs which are sending directly to the controller
will
rely on forwarding as well.". I suggest to explicitly mention how "old
admin clients"
will work here to complement the sentence. Something like: They will get a
random
broker id as the controller id in the metadata response and stick to it as
you explained.

- "The purpose of adding principal name is for the audit logging, and the
client id is
being used to throttling according to KIP-599 requirement." Actually,
KIP-599 needs
both the principal and the clientId.

- In the "Routing Request Security" chapter. It is written that the
forwarding broker
will verify the request with its own authorizer and will just forward it if
the request
looks good. When a request contains for instance multiple topics, I suppose
that
we will forward only the authorized ones and not the whole original request
as is.
We may want to reword the sentence to make this clear.

- For the record, should we put the previous proposal in the rejected
alternatives as
well?

Best,
David

On Thu, Jul 30, 2020 at 3:51 AM Boyang Chen 
wrote:

> Thanks David for the feedback!
>
> On Wed, Jul 29, 2020 at 7:53 AM David Jacot  wrote:
>
> > Hi, Colin, Boyang,
> >
> > Colin, thanks for the clarification. Somehow, I thought that even if the
> > controller is ran independently, it
> > would still run the listeners of the broker and thus would be accessible
> by
> > redirecting on the loopback
> > interface. My mistake.
> >
> > Boyang, I have few questions/comments regarding the updated KIP:
> >
> > 1. I think that it would be great if we could clarify how old admin
> clients
> > which are directly talking to the
> > controller will work with this KIP. I read between the lines that, as we
> > propose to provide a random
> > broker Id as the controller Id in the metadata response, they will use a
> > single node as a proxy. Is that
> > correct? This deserves to be called out more explicitly in the design
> > section instead of being hidden
> > in the protocol bump of the metadata RPC.
> >
> > Makes sense, I stress this point in the compatibility section.
>
>
> > 1.1 If I understand correctly, could we assume that old admin clients
> will
> > stick to the same "fake controller"
> > until they refresh their metadata? Refreshing the metadata usually
> happens
> > when NOT_CONTROLLER
> > is received but this won't happen anymore so they should change
> > infrequently.
> >
> > That is correct, old admin clients would not try to refresh their
> metadata
> due to NOT_CONTROLLER,
> which is impossible to happen with the new broker cluster.
>
>
> > 2. For the new admin client, I suppose that we plan on using
> > LeastLoadedNodeProvider for the
> > requests that are using ControllerNodeProvider. We could perhaps mention
> > it.
> >
> > Sure, added.
>
>
> > 3. Pre KIP-500, will we have a way to distinguish if a request that is
> > received by the controller is
> > coming directly from a client or from a broker? You mention that the
> > listener can be used to do
> > this but as you pointed out, it is not mandatory. Do we have another
> > reliable method? I am asking
> > in the context of KIP-599 with the current controller, we may need to
> > throttle differently if the
> > request comes from a client or from a broker.
> >
> > The point for using the listener name is more of a security purpose, to
> de

Re: [VOTE] KIP-635: GetOffsetShell: support for multiple topics and consumer configuration override

2020-07-30 Thread David Jacot
Hi Daniel,

Thanks for the KIP.

It seems that we have forgotten to include this tool in KIP-499. KAFKA-8507
is resolved
by this tool still uses the deprecated "--broker-list". I suggest to
include "--bootstrap-server"
in your public interfaces as well and fix this omission during the
implementation.

+1 (non-binding)

Thanks,
David

On Thu, Jul 30, 2020 at 1:52 PM Kamal Chandraprakash <
kamal.chandraprak...@gmail.com> wrote:

> +1 (non-binding), thanks for the KIP!
>
> On Thu, Jul 30, 2020 at 3:31 PM Manikumar 
> wrote:
>
> > +1 (binding)
> >
> > Thanks for the KIP!
> >
> >
> >
> > On Thu, Jul 30, 2020 at 3:07 PM Dániel Urbán 
> > wrote:
> >
> > > Hi everyone,
> > >
> > > If you are interested in this KIP, please do not forget to vote.
> > >
> > > Thanks,
> > > Daniel
> > >
> > > Viktor Somogyi-Vass  ezt írta (időpont: 2020.
> > > júl.
> > > 28., K, 16:06):
> > >
> > > > +1 from me (non-binding), thanks for the KIP.
> > > >
> > > > On Mon, Jul 27, 2020 at 10:02 AM Dániel Urbán  >
> > > > wrote:
> > > >
> > > > > Hello everyone,
> > > > >
> > > > > I'd like to start a vote on KIP-635. The KIP enhances the
> > > GetOffsetShell
> > > > > tool by enabling querying multiple topic-partitions, adding new
> > > filtering
> > > > > options, and adding a config override option.
> > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-635%3A+GetOffsetShell%3A+support+for+multiple+topics+and+consumer+configuration+override
> > > > >
> > > > > The original discussion thread was named "[DISCUSS] KIP-308:
> > > > > GetOffsetShell: new KafkaConsumer API, support for multiple topics,
> > > > > minimize the number of requests to server". The id had to be
> changed
> > as
> > > > > there was a collision, and the KIP also had to be renamed, as some
> of
> > > its
> > > > > motivations were outdated.
> > > > >
> > > > > Thanks,
> > > > > Daniel
> > > > >
> > > >
> > >
> >
>


Re: [VOTE] KIP-599: Throttle Create Topic, Create Partition and Delete Topic Operations

2020-08-06 Thread David Jacot
Hi all,

I would like to inform you that we have slightly changed our thoughts about
the implementation
of the Token Bucket algorithm. Our initial idea was to change our existing
Rate to behave like
a Token Bucket. That works as we expected but we have realized that the
value of the Rate is
not really a sampled Rate anymore. From an observability perspective, it is
not good to change
the behavior of our Rate. Instead, we propose to implement a new
MeasurableStat that implements
the Token Bucket algorithm and to use it alongside the Rate in the Sensor.
Which this, the Rate
remains there for observability purposes and the Token Bucket is used for
enforcing the quota.
The Token Bucket metrics is exposed via a new metric named "credits" that
represents the
remaining amount of credits in the bucket. As a first step, this will be
only used for the controller
quota.

I have updated the KIP to reflect this change.

We hope this change is fine for everyone. Please, raise your concerns if
not.

Best,
David

On Tue, Jul 28, 2020 at 1:48 PM David Jacot  wrote:

> Hi all,
>
> Just a quick update. We have made good progress regarding the
> implementation
> of this KIP. The major parts are already in trunk modulo the new "rate"
> implementation
> which is still under development.
>
> I would like to change the type of the `controller_mutations_rate` from a
> Long to
> a Double. I have made various experiments and the rate can be quite low,
> especially
> in clusters with a lot of tenants. Using a Long is quite limiting here to
> fine tune small
> rates. I hope that this change is fine for everyone.
>
> Best,
> David
>
> On Mon, Jun 15, 2020 at 9:26 AM David Jacot  wrote:
>
>> Hi all,
>>
>> The vote has passed with 5 binding votes (Gwen, Rajini, Mickael, Jun and
>> Colin)
>> and 2 non-binding votes (Tom, Anna).
>>
>> Thank you all for the fruitful discussion! I'd like to particularly thank
>> Anna who has
>> heavily contributed to the design of this KIP.
>>
>> Regards,
>> David
>>
>> On Fri, Jun 12, 2020 at 10:08 PM Colin McCabe  wrote:
>>
>>> +1.  Thanks, David!
>>>
>>> best,
>>> Colin
>>>
>>> On Thu, Jun 11, 2020, at 23:51, David Jacot wrote:
>>> > Colin, Jun,
>>> >
>>> > Do the proposed error code and the updated KIP look good to you guys?
>>> I’d
>>> > like to wrap up and close the vote.
>>> >
>>> > Thanks,
>>> > David
>>> >
>>> > Le mer. 10 juin 2020 à 14:50, David Jacot  a
>>> écrit :
>>> >
>>> > > Hi Colin and Jun,
>>> > >
>>> > > I have no problem if we have to rewrite part of it when the new
>>> controller
>>> > > comes
>>> > > out. I will be more than happy to help out.
>>> > >
>>> > > Regarding KIP-590, I think that we can cope with a principal as a
>>> string
>>> > > when the
>>> > > time comes. The user entity name is defined with a string already.
>>> > >
>>> > > Regarding the name of the error, you have made a good point. I do
>>> agree
>>> > > that it
>>> > > is important to differentiate the two cases. I propose the following
>>> two
>>> > > errors:
>>> > > - THROTTLING_QUOTA_EXCEEDED - Throttling is slightly better than
>>> rate as
>>> > > we have quotas which are not rate (e.g. request quota). This one is
>>> > > retryable
>>> > > once the throttle time is passed.
>>> > > - LIMIT_QUOTA_EXCEEDED - This one would indicate that the limit has
>>> been
>>> > > reached and is a final error.
>>> > > We only need the former in this KIP. What do you think?
>>> > >
>>> > > Jun, I have added a few examples in the KIP. The new name works
>>> exactly
>>> > > like
>>> > > the existing one once it is added to the accepted dynamic configs
>>> for the
>>> > > user
>>> > > and the client entities. I have added a "Kafka Config Command"
>>> chapter in
>>> > > the
>>> > > KIP. I will also open a Jira to not forget updating the AK
>>> documentation
>>> > > once
>>> > > the KIP gets merged.
>>> > >
>>> > > Thanks,
>>> > > David
>>> > >
>>> > > On Wed, Jun 10, 2020 at 3:03 AM Jun Rao  wrote:
>>> > >
>>> > &g

Re: [VOTE] KIP-651 - Support PEM format for SSL certificates and private key

2020-08-06 Thread David Jacot
Supporting PEM is really nice. Thanks, Rajini.

+1 (non-binding)

On Thu, Aug 6, 2020 at 9:18 PM Gwen Shapira  wrote:

> +1 (binding)
> Thank you for driving this, Rajini
>
> On Thu, Aug 6, 2020 at 10:43 AM Rajini Sivaram 
> wrote:
>
> > Hi all,
> >
> > I would like to start vote on KIP-651 to support SSL key stores and trust
> > stores in PEM format:
> >
> >-
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-651+-+Support+PEM+format+for+SSL+certificates+and+private+key
> >
> >
> > Thank you...
> >
> > Regards,
> >
> > Rajini
> >
>
>
> --
> Gwen Shapira
> Engineering Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter | blog
>


Re: [VOTE] KIP-635: GetOffsetShell: support for multiple topics and consumer configuration override

2020-08-10 Thread David Jacot
Hi Daniel,

I was not aware of that PR. At minimum, I would add `--bootstrap-server`
to the list in the KIP for completeness. Regarding the implementation,
I would leave a comment in that PR asking if they plan to continue it. If
not,
we could do it as part of your PR directly.

Cheers,
David

On Mon, Aug 10, 2020 at 10:49 AM Dániel Urbán  wrote:

> Hi everyone,
>
> Just a reminder, please vote if you are interested in this KIP being
> implemented.
>
> Thanks,
> Daniel
>
> Dániel Urbán  ezt írta (időpont: 2020. júl. 31., P,
> 9:01):
>
> > Hi David,
> >
> > There is another PR linked on KAFKA-8507, which is still open:
> > https://github.com/apache/kafka/pull/8123
> > Wasn't sure if it will go in, and wanted to avoid conflicts. Do you think
> > I should do the switch to '--bootstrap-server' anyway?
> >
> > Thanks,
> > Daniel
> >
> > David Jacot  ezt írta (időpont: 2020. júl. 30., Cs,
> > 17:52):
> >
> >> Hi Daniel,
> >>
> >> Thanks for the KIP.
> >>
> >> It seems that we have forgotten to include this tool in KIP-499.
> >> KAFKA-8507
> >> is resolved
> >> by this tool still uses the deprecated "--broker-list". I suggest to
> >> include "--bootstrap-server"
> >> in your public interfaces as well and fix this omission during the
> >> implementation.
> >>
> >> +1 (non-binding)
> >>
> >> Thanks,
> >> David
> >>
> >> On Thu, Jul 30, 2020 at 1:52 PM Kamal Chandraprakash <
> >> kamal.chandraprak...@gmail.com> wrote:
> >>
> >> > +1 (non-binding), thanks for the KIP!
> >> >
> >> > On Thu, Jul 30, 2020 at 3:31 PM Manikumar 
> >> > wrote:
> >> >
> >> > > +1 (binding)
> >> > >
> >> > > Thanks for the KIP!
> >> > >
> >> > >
> >> > >
> >> > > On Thu, Jul 30, 2020 at 3:07 PM Dániel Urbán  >
> >> > > wrote:
> >> > >
> >> > > > Hi everyone,
> >> > > >
> >> > > > If you are interested in this KIP, please do not forget to vote.
> >> > > >
> >> > > > Thanks,
> >> > > > Daniel
> >> > > >
> >> > > > Viktor Somogyi-Vass  ezt írta (időpont:
> >> 2020.
> >> > > > júl.
> >> > > > 28., K, 16:06):
> >> > > >
> >> > > > > +1 from me (non-binding), thanks for the KIP.
> >> > > > >
> >> > > > > On Mon, Jul 27, 2020 at 10:02 AM Dániel Urbán <
> >> urb.dani...@gmail.com
> >> > >
> >> > > > > wrote:
> >> > > > >
> >> > > > > > Hello everyone,
> >> > > > > >
> >> > > > > > I'd like to start a vote on KIP-635. The KIP enhances the
> >> > > > GetOffsetShell
> >> > > > > > tool by enabling querying multiple topic-partitions, adding
> new
> >> > > > filtering
> >> > > > > > options, and adding a config override option.
> >> > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-635%3A+GetOffsetShell%3A+support+for+multiple+topics+and+consumer+configuration+override
> >> > > > > >
> >> > > > > > The original discussion thread was named "[DISCUSS] KIP-308:
> >> > > > > > GetOffsetShell: new KafkaConsumer API, support for multiple
> >> topics,
> >> > > > > > minimize the number of requests to server". The id had to be
> >> > changed
> >> > > as
> >> > > > > > there was a collision, and the KIP also had to be renamed, as
> >> some
> >> > of
> >> > > > its
> >> > > > > > motivations were outdated.
> >> > > > > >
> >> > > > > > Thanks,
> >> > > > > > Daniel
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>


Re: [VOTE] KIP-612: Ability to Limit Connection Creation Rate on Brokers

2020-09-02 Thread David Jacot
Hi Anna,

Thanks for the update.

If you use Token Bucket, it will expose another metric which reports the
number of remaining tokens in the bucket, in addition to the current rate
metric. It would be great to add it in the metrics section of the KIP as
well
for completeness.

Best,
David

On Tue, Aug 11, 2020 at 4:28 AM Anna Povzner  wrote:

> Hi All,
>
> I wanted to let everyone know that we would like to make the following
> changes to the KIP:
>
>1.
>
>Expose connection acceptance rate metrics (broker-wide and per-listener)
>and per-listener average throttle time metrics for better observability
> and
>debugging.
>2.
>
>KIP-599 introduced a new implementation of MeasurableStat that
>implements a token bucket, which improves rate throttling for bursty
>workloads (KAFKA-10162). We would like to use this same mechanism for
>connection accept rate throttling.
>
>
> I updated the KIP to reflect these changes.
>
> Let me know if you have any concerns.
>
> Thanks,
>
> Anna
>
>
> On Thu, May 21, 2020 at 5:42 PM Anna Povzner  wrote:
>
> > The vote for KIP-612 has passed with 3 binding and 3 non-binding +1s, and
> > no objections.
> >
> >
> > Thanks everyone for reviews and feedback,
> >
> > Anna
> >
> > On Tue, May 19, 2020 at 2:41 AM Rajini Sivaram 
> > wrote:
> >
> >> +1 (binding)
> >>
> >> Thanks for the KIP, Anna!
> >>
> >> Regards,
> >>
> >> Rajini
> >>
> >>
> >> On Tue, May 19, 2020 at 9:32 AM Alexandre Dupriez <
> >> alexandre.dupr...@gmail.com> wrote:
> >>
> >> > +1 (non-binding)
> >> >
> >> > Thank you for the KIP!
> >> >
> >> >
> >> > Le mar. 19 mai 2020 à 07:57, David Jacot  a
> écrit
> >> :
> >> > >
> >> > > +1 (non-binding)
> >> > >
> >> > > Thanks for the KIP, Anna!
> >> > >
> >> > > On Tue, May 19, 2020 at 7:12 AM Satish Duggana <
> >> satish.dugg...@gmail.com
> >> > >
> >> > > wrote:
> >> > >
> >> > > > +1 (non-binding)
> >> > > > Thanks Anna for the nice feature to control the connection
> creation
> >> > rate
> >> > > > from the clients.
> >> > > >
> >> > > > On Tue, May 19, 2020 at 8:16 AM Gwen Shapira 
> >> > wrote:
> >> > > >
> >> > > > > +1 (binding)
> >> > > > >
> >> > > > > Thank you for driving this, Anna
> >> > > > >
> >> > > > > On Mon, May 18, 2020 at 4:55 PM Anna Povzner  >
> >> > wrote:
> >> > > > >
> >> > > > > > Hi All,
> >> > > > > >
> >> > > > > > I would like to start the vote on KIP-612: Ability to limit
> >> > connection
> >> > > > > > creation rate on brokers.
> >> > > > > >
> >> > > > > > For reference, here is the KIP wiki:
> >> > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> >
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-612%3A+Ability+to+Limit+Connection+Creation+Rate+on+Brokers
> >> > > > > >
> >> > > > > > And discussion thread:
> >> > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> >
> >>
> https://lists.apache.org/thread.html/r61162661fa307d0bc5c8326818bf223a689c49e1c828c9928ee26969%40%3Cdev.kafka.apache.org%3E
> >> > > > > >
> >> > > > > > Thanks,
> >> > > > > >
> >> > > > > > Anna
> >> > > > > >
> >> > > > >
> >> > > > >
> >> > > > > --
> >> > > > > Gwen Shapira
> >> > > > > Engineering Manager | Confluent
> >> > > > > 650.450.2760 | @gwenshap
> >> > > > > Follow us: Twitter | blog
> >> > > > >
> >> > > >
> >> >
> >>
> >
>


Re: [DISCUSS] KIP-516: Topic Identifiers

2020-09-24 Thread David Jacot
Hi Justine,

Thanks for the KIP. I finally had time to read it :). It is a great
improvement.

I have a few comments/questions:

1. It seems that the schema of the StopReplicaRequest is slightly outdated.
We
did some changes as part of KIP-570. V3 is already organized by topics.

2. I just want to make sure that I understand the reconciliation
logic correctly. When
an "INCREMENTAL" LeaderAndIsr Request is received, the broker will also
reconcile
when the local uuid does not match the uuid in the request, right? In this
case, the
local log is staged for deletion.

3. In the documentation of the `delete.stale.topic.delay.ms` config, it
says "When a
FULL LeaderAndIsrRequest is received..." but I suppose it applies to both
types.

Best,
David

On Thu, Sep 24, 2020 at 1:40 AM Justine Olshan  wrote:

> Hi Jun,
>
> Thanks for the comments. I apologize for some of the typos and confusion.
> I’ve updated the KIP to fix some of the issues you mentioned.
>
> 20.2 I’ve changed the type to String.
> 20.1/3 I’ve updated the TopicZNode to fix formatting and reflect the latest
> version before this change.
>
> 21. You are correct and I’ve removed this line. I’ve also added a line
> mentioning an IBP bump is necessary for migration
>
> 22. I think the wording was unclear but your summary is what was intended
> by this line. I’ve updated to make this point more clear. “Remove deleted
> topics from replicas by sending StopReplicaRequest V3 before the IBP bump
> using the old logic, and using V4 and the new logic with topic IDs after
> the IBP bump.”
>
> 23. I’ve removed the unspecified type from the KIP and mention that IBP
> will be used to handle this request. “IBP will be used to determine whether
> this new form of the request will be used. For older requests, we will
> ignore this field and default to previous behavior.”
>
> 24. I’ve fixed this typo.
>
> 25. I've added a topics at a higher level for LeaderAndIsrResponse v5,
> StopReplicaResponse v4. I've also changed StopReplicaRequest v4 to have
> topics at a higher level.
>
> 26. I’ve updated forgotten_topics_data--added the topic ID and removed the
> topic name
>
> 27. I’ve decided on plain text, and I’ve added an example.
>
> 28. I’ve added this idea to future work.
>
> Thanks again for taking a look,
>
> Justine
>
> On Wed, Sep 23, 2020 at 10:28 AM Jun Rao  wrote:
>
> > Hi, Justine,
> >
> > Thanks for the response. Made another pass. A few more comments below.
> >
> > 20. znode schema:
> > 20.1 It seems that {"name": "version", "type": "int", "id": "UUID",
> "doc":
> > "version id"} should be {"name": "version", "type": "int"}, {"name":
> "id",
> > "type": "UUID", "doc": "version id"}.
> > 20.2 The znode format is JSON which doesn't have UUID type. So the type
> > probably should be string?
> > 20.3 Also, the existing format used seems outdated. It should have the
> > following format.
> > Json.encodeAsBytes(Map(
> >   "version" -> 2,
> >   "partitions" -> replicaAssignmentJson.asJava,
> >   "adding_replicas" -> addingReplicasAssignmentJson.asJava,
> >   "removing_replicas" -> removingReplicasAssignmentJson.asJava
> > ).asJava)
> >   }
> >
> > 21. Migration: The KIP says "The migration process can take place without
> > an inter-broker protocol bump, as the format stored in
> > /brokers/topics/[topic] will be compatible with older broker versions."
> > However, since we are bumping up the version of inter-broker requests, it
> > seems that we need to use IBP for migration.
> >
> > 22. The KIP says "Remove deleted topics from replicas by sending
> > StopReplicaRequest V3 for any topics which do not contain a topic ID, and
> > V4 for any topics which do contain a topic ID.". However, if we use IBP,
> it
> > seems that the controller will either send StopReplicaRequest V3
> > or StopReplicaRequest V4, but never mixed V3 and V4 for different topics.
> > Basically, before the IBP bump, V3 will be used. After the IBP bump,
> > topicId will be created and V4 will be used.
> >
> > 23. Given that we depend on IBP, do we still need "0 UNSPECIFIED"
> > in LeaderAndIsr?
> >
> > 24. LeaderAndIsrResponse v5 : It still has the topic field.
> >
> > 25. LeaderAndIsrResponse v5, StopReplicaResponse v4: Could we use this
> > opportunity to organize the response in 2 levels, first by topic, then by
> > partition, as most other requests/responses?
> >
> > 26. FetchRequest v13 : Should forgotten_topics_data use topicId too?
> >
> > 27. "This file can either be plain text (key/value pairs) or JSON." Have
> we
> > decided which one to use? Also, it would be helpful to provide an
> example.
> >
> > 28. Future improvement: Another future improvement opportunity is to use
> > topicId in GroupMetadataManager.offsetCommitKey in the offset_commit
> topic.
> > This may save some space.
> >
> > Thanks,
> >
> > Jun
> >
> > On Wed, Sep 23, 2020 at 8:50 AM Justine Olshan 
> > wrote:
> >
> > > Hi Tom,
> > >
> > > Thanks for the comment. I think this is a really good idea and i

Re: Complete Kafka replication protocol description

2023-09-10 Thread David Jacot
Hi Jack,

This is great! Thanks for doing it. I will look into it when I have a bit
of time, likely after Current.

Would you be interested in contributing it to the main repository? That
would be a great contribution to the project. Having it there would allow
the community to maintain it while changes to the protocol are made. That
would also pave the way for having other specs in the future (e.g. new
consumer group protocol).

Best,
David

Le dim. 10 sept. 2023 à 12:45, Jack Vanlightly  a
écrit :

> Hi all,
>
> As part of my work on formally verifying different parts of Apache Kafka
> and working on KIP-966 I have built up a lot of knowledge about how the
> replication protocol works. Currently it is mostly documented across
> various KIPs and in the code itself. I have written a complete protocol
> description (with KIP-966 changes applied) which is inspired by the precise
> but accessible style and language of the Raft paper. The idea is that it
> makes it easier for contributors and anyone else interested in the protocol
> to learn how it works, the fundamental properties it has and how those
> properties are supported by the various behaviors and conditions.
>
> It currently resides next to the TLA+ specification itself in my
> kafka-tlaplus repository. I'd be interested to receive feedback from the
> community.
>
>
> https://github.com/Vanlightly/kafka-tlaplus/blob/main/kafka_data_replication/kraft/kip-966/description/0_kafka_replication_protocol.md
>
> Thanks
> Jack
>


Re: Re: [DISCUSS] KIP-951: Leader discovery optimisations for the client

2023-09-28 Thread David Jacot
Hi Mayank,

Thanks again for the KIP and thanks for adding the new analysis. Overall, I
am fine with it. I have a few minor comments.

01. If I understand correctly, the client will still request a metadata
update even when it gets the new leader if the produce response or the
fetch response. Is this correct? I think that we need this in order to get
the full metadata. Could we elaborate a bit more about this in the KIP? I
mean that it would be great to explain why we are doing it.

02. I was debating whether we should return the rack in the NodeEndpoints.
I think that returning it makes sense from a consistency point of view. It
would be weird to only update the node and the port in the metadata cache.
However, I was thinking about the future and I was wondering how we would
handle future metadata that we would add to the nodes, say tags. If we
follow the same pattern, we would have to return any new fields as well. I
suppose that it would be fine. Anyway, I think that the current approach is
OK. I just wanted to share my thoughts.

03. In the FetchResponse Message section, it is written "NodeEndpoints is a
tagged field, which is a minor optimisation of saving bytes on the network,
as it won’t be set always.". Are you saying this because it won't be set
for version prior to version 16? Or are you saying this because it may not
be provided even in version 16? In other words, I wonder if that new
information is actually optional or not. It is not clear in the KIP.

04. In the ProduceResponse Message section, the validVersions field misses
in the schema.

05. The documentation of NodeEndpoints fields says "Endpoints for all
current-leaders enumerated in PartitionData.". I suppose that this is
incorrect, right? We will only provide endpoints
when NOT_LEADER_OR_FOLLOWER or FENCED_LEADER_EPOCH are returned. The same
applies to the other schema.

Best,
David


On Wed, Sep 27, 2023 at 4:39 AM Mayank Shekhar Narula <
mayanks.nar...@gmail.com> wrote:

> Adding to Crispin. The new micro-benchmark shows improvements of 88% in
> p99.9 with the KIP changes Vs baseline(& rejected alternate). Its
> hypothesised improvements are seen as KIP avoids a full metadata refresh(Vs
> baseline/rejected alternate), and the full metadata refresh can be slow due
> to metadata reconvergence delay at the server(post leadership-change of
> partitions). Extending this logic, KIP changes would be beneficial in
> scenarios where full metadata refresh can be slow. Potential example would
> be, metadata RPC is slowed due to head-of-line blocking by another slow RPC
> in front, say a produce RPC(possibly slow due to churn in the ISR).
>
> This new benchmark is in addition to the previously done benchmark of roll
> simulation, where improvements upto 5% were seen.
>
> Please do a review, as the voting thread is live.
>
> Thanks!
>
> On Wed, Sep 20, 2023 at 4:43 PM Crispin Bernier
>  wrote:
>
> > Hi,
> >
> > I've updated the KIP with benchmark results focusing more directly on the
> > redirect case, please review and +1 in the voting thread if convinced.
> >
> > Thank you,
> > Crispin
> >
> > On Wed, Jul 26, 2023 at 11:13 PM Luke Chen  wrote:
> >
> > > Thanks for adding the benchmark results, Crispin!
> > > IMO, 2~5% performance improvement is small, but given the change is
> > small,
> > > cost is also small (only append endpoint info when
> > NOT_LEADER_OR_FOLLOWER..
> > > etc error), I think it is worth doing it.
> > >
> > > Thank you.
> > > Luke
> > >
> > > On Wed, Jul 26, 2023 at 12:33 AM Ismael Juma 
> wrote:
> > >
> > > > Thanks Crispin!
> > > >
> > > > Ismael
> > > >
> > > > On Mon, Jul 24, 2023 at 8:16 PM Crispin Bernier
> > > >  wrote:
> > > >
> > > > > I updated the wiki to include both results along with their
> average.
> > > > >
> > > > > Thank you,
> > > > > Crispin
> > > > >
> > > > > On Mon, Jul 24, 2023 at 10:58 AM Ismael Juma 
> > > wrote:
> > > > >
> > > > > > Hi Crispin,
> > > > > >
> > > > > > One additional question, the wiki says "The results are averaged
> > > over 2
> > > > > > runs.". Can you please provide some measure of variance in the
> > > > > > distribution, i.e. were both results similar to each other for
> both
> > > > > cases?
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Fri, Jul 21, 2023 at 11:31 AM Ismael Juma 
> > > > wrote:
> > > > > >
> > > > > > > Thanks for the update Crispin - very helpful to have actual
> > > > performance
> > > > > > > data. 2-5% for the default configuration is a bit on the low
> side
> > > for
> > > > > > this
> > > > > > > kind of proposal.
> > > > > > >
> > > > > > > Ismael
> > > > > > >
> > > > > > > On Thu, Jul 20, 2023 at 11:33 PM Crispin Bernier
> > > > > > >  wrote:
> > > > > > >
> > > > > > >> Benchmark numbers have been posted on the KIP, please review.
> > > > > > >>
> > > > > > >> On 2023/07/20 13:03:00 Mayank Shekhar Narula wrote:
> > > > > > >> > Jun
> > > > > > >> >
> > > > > > >> > Thanks for the feedback.
> > > > > > >> >
> > > > > > >> > Numbers to fo

Re: [DISCUSS] KIP-714: Client metrics and observability

2023-09-29 Thread David Jacot
Hi Andrew,

Thanks for driving this one. I haven't read all the KIP yet but I already
have an initial question. In the Threading section, it is written
"KafkaConsumer: the "background" thread (based on the consumer threading
refactor which is underway)". If I understand this correctly, it means
that KIP-714 won't work if the "old consumer" is used. Am I correct?

Cheers,
David


On Fri, Sep 22, 2023 at 12:18 PM Andrew Schofield <
andrew_schofield_j...@outlook.com> wrote:

> Hi Philip,
> No, I do not think it should actively search for a broker that supports
> the new
> RPCs. In general, either all of the brokers or none of the brokers will
> support it.
> In the window, where the cluster is being upgraded or client telemetry is
> being
> enabled, there might be a mixed situation. I wouldn’t put too much effort
> into
> this mixed scenario. As the client finds brokers which support the new
> RPCs,
> it can begin to follow the KIP-714 mechanism.
>
> Thanks,
> Andrew
>
> > On 22 Sep 2023, at 20:01, Philip Nee  wrote:
> >
> > Hi Andrew -
> >
> > Question on top of your answers: Do you think the client should actively
> > search for a broker that supports this RPC? As previously mentioned, the
> > broker uses the leastLoadedNode to find its first connection (am
> > I correct?), and what if that broker doesn't support the metric push?
> >
> > P
> >
> > On Fri, Sep 22, 2023 at 10:20 AM Andrew Schofield <
> > andrew_schofield_j...@outlook.com> wrote:
> >
> >> Hi Kirk,
> >> Thanks for your question. You are correct that the presence or absence
> of
> >> the new RPCs in the
> >> ApiVersionsResponse tells the client whether to request the telemetry
> >> subscriptions and push
> >> metrics.
> >>
> >> This is of course tricky in practice. It would be conceivable, as a
> >> cluster is upgraded to AK 3.7
> >> or as a client metrics receiver plugin is deployed across the cluster,
> >> that a client connects to some
> >> brokers that support the new RPCs and some that do not.
> >>
> >> Here’s my suggestion:
> >> * If a client is not connected to any brokers that support in the new
> >> RPCs, it cannot push metrics.
> >> * If a client is only connected to brokers that support the new RPCs, it
> >> will use the new RPCs in
> >> accordance with the KIP.
> >> * If a client is connected to some brokers that support the new RPCs and
> >> some that do not, it will
> >> use the new RPCs with the supporting subset of brokers in accordance
> with
> >> the KIP.
> >>
> >> Comments?
> >>
> >> Thanks,
> >> Andrew
> >>
> >>> On 22 Sep 2023, at 16:01, Kirk True  wrote:
> >>>
> >>> Hi Andrew/Jun,
> >>>
> >>> I want to make sure I understand question/comment #119… In the case
> >> where a cluster without a metrics client receiver is later reconfigured
> and
> >> restarted to include a metrics client receiver, do we want the client to
> >> thereafter begin pushing metrics to the cluster? From Andrew’s response
> to
> >> question #119, it sounds like we’re using the presence/absence of the
> >> relevant RPCs in ApiVersionsResponse as the to-push-or-not-to-push
> >> indicator. Do I have that correct?
> >>>
> >>> Thanks,
> >>> Kirk
> >>>
>  On Sep 21, 2023, at 7:42 AM, Andrew Schofield <
> >> andrew_schofield_j...@outlook.com> wrote:
> 
>  Hi Jun,
>  Thanks for your comments. I’ve updated the KIP to clarify where
> >> necessary.
> 
>  110. Yes, agree. The motivation section mentions this.
> 
>  111. The replacement of ‘-‘ with ‘.’ for metric names and the
> >> replacement of
>  ‘-‘ with ‘_’ for attribute keys is following the OTLP guidelines. I
> >> think it’s a bit
>  of a debatable point. OTLP makes a distinction between a namespace
> and a
>  multi-word component. If it was “client.id” then “client” would be a
> >> namespace with
>  an attribute key “id”. But “client_id” is just a key. So, it was
> >> intentional, but debatable.
> 
>  112. Thanks. The link target moved. Fixed.
> 
>  113. Thanks. Fixed.
> 
>  114.1. If a standard metric makes sense for a client, it should use
> the
> >> exact same
>  name. If a standard metric doesn’t make sense for a client, then it
> can
> >> omit that metric.
> 
>  For a required metric, the situation is stronger. All clients must
> >> implement these
>  metrics with these names in order to implement the KIP. But the
> >> required metrics
>  are essentially the number of connections and the request latency,
> >> which do not
>  reference the underlying implementation of the client (which
> >> producer.record.queue.time.max
>  of course does).
> 
>  I suppose someone might build a producer-only client that didn’t have
> >> consumer metrics.
>  In this case, the consumer metrics would conceptually have the value 0
> >> and would not
>  need to be sent to the broker.
> 
>  114.2. If a client does not implement some metrics, they will not be
> >> available for
>  analysis and troubles

Re: [VOTE] KIP-951: Leader discovery optimisations for the client

2023-10-03 Thread David Jacot
Thanks for the KIP. +1 from me as well.

Best,
David

Le mar. 3 oct. 2023 à 20:54, Jun Rao  a écrit :

> Hi, Mayank,
>
> Thanks for the detailed explanation in the KIP. +1 from me.
>
> Jun
>
> On Wed, Sep 27, 2023 at 4:39 AM Mayank Shekhar Narula <
> mayanks.nar...@gmail.com> wrote:
>
> > Reviving this thread, as the discussion thread has been updated.
> >
> > On Fri, Jul 28, 2023 at 11:29 AM Mayank Shekhar Narula <
> > mayanks.nar...@gmail.com> wrote:
> >
> > > Thanks Jose.
> > >
> > > On Thu, Jul 27, 2023 at 5:46 PM José Armando García Sancio
> > >  wrote:
> > >
> > >> The KIP LGTM. Thanks for the design. I am looking forward to the
> > >> implementation.
> > >>
> > >> +1 (binding).
> > >>
> > >> Thanks!
> > >> --
> > >> -José
> > >>
> > >
> > >
> > > --
> > > Regards,
> > > Mayank Shekhar Narula
> > >
> >
> >
> > --
> > Regards,
> > Mayank Shekhar Narula
> >
>


Re: [DISCUSS] KIP-966: Eligible Leader Replicas

2023-10-04 Thread David Jacot
Hi Calvin,

I thought that a new snapshot with the downgraded MV is created in this
case. Isn’t it the case?

Could you also elaborate a bit more on the reasoning behind adding the
limits to the admin RPCs? This is a new pattern in Kafka so it would be
good to clear on the motivation.

Could you also explain how the client is supposed to handle the
topics/partitions above the limit? I suppose that it will have to retry
those, correct?

My understanding is that the topics/partitions above the limit will be
failed with an invalid exception error. I wonder if this choice is
judicious because the invalide request exception is usually fatal. It may
be better to use an new and explicit error for this case.

It seems that we still need to specify the changes to the admin api to
accommodate the new or updated apis. Do you plan to add them?

Best,
David

Le mer. 4 oct. 2023 à 20:39, Calvin Liu  a
écrit :

> Hi Jun,
> After the MV downgrade, the controller will write in the old version of the
> PartitionRecord/PartitionChangeRecord. If I understand correctly, it is
> possible to downgrade the software version if the controller only has to
> handle old version records.
> However, the controller will not automatically rewrite the PartitionRecord
> with the old version unless there is a partition update. Then, the user may
> have to wait an unknown amount of time before the software downgrades
> unless they do a roll to force update every partition. If it makes sense, I
> can mention these steps to do a software downgrade.
> Thanks
>
> On Wed, Oct 4, 2023 at 11:20 AM Jun Rao  wrote:
>
> > Hi, Calvin and Justine,
> >
> > Historically, when we change the record format in the log, we don't
> support
> > software version downgrading.
> >
> > For the record format change in the metadata log, have we thought about
> > forcing the write of the latest metadata records with the old version
> > during MV downgrading? This will in theory allow the old version of the
> > software to obtain the latest metadata.
> >
> > Thanks,
> >
> > Jun
> >
> > On Wed, Oct 4, 2023 at 9:53 AM Justine Olshan
>  > >
> > wrote:
> >
> > > Sorry -- not MV but software version.
> > >
> > > On Wed, Oct 4, 2023 at 9:51 AM Justine Olshan 
> > > wrote:
> > >
> > > > Catching up with this discussion.
> > > >
> > > > I was just curious -- have we had other instances where downgrading
> MV
> > is
> > > > not supported? I think Kafka typically tries to support downgrades,
> > and I
> > > > couldn't think of other examples.
> > > >
> > > > Thanks,
> > > > Justine
> > > >
> > > > On Wed, Oct 4, 2023 at 9:40 AM Calvin Liu  >
> > > > wrote:
> > > >
> > > >> Hi Jun,
> > > >> 54. Marked the software downgrading is not supported. As the old
> > > >> controller
> > > >> will not understand the new PartitionRecord and
> PartitionChangeRecord.
> > > >> Thanks!
> > > >>
> > > >> On Wed, Oct 4, 2023 at 9:12 AM Jun Rao 
> > > wrote:
> > > >>
> > > >> > Hi, Calvin,
> > > >> >
> > > >> > Thanks for the reply. Just one more comment.
> > > >> >
> > > >> > 54. It seems that downgrading MV is supported. Is downgrading the
> > > >> software
> > > >> > version supported? It would be useful to document that.
> > > >> >
> > > >> > Thanks,
> > > >> >
> > > >> > Jun
> > > >> >
> > > >> > On Tue, Oct 3, 2023 at 4:55 PM Artem Livshits
> > > >> >  wrote:
> > > >> >
> > > >> > > Hi Colin,
> > > >> > >
> > > >> > > I think in your example "do_unclean_recovery" would need to do
> > > >> different
> > > >> > > things depending on the strategy.
> > > >> > >
> > > >> > > do_unclean_recovery() {
> > > >> > >if (unclean.recovery.manager.enabled) {
> > > >> > > if (strategy == Aggressive)
> > > >> > >   use UncleanRecoveryManager(waitLastKnownERL=false)  //
> just
> > > >> inspect
> > > >> > > logs from whoever is available
> > > >> > > else
> > > >> > >   use  UncleanRecoveryManager(waitLastKnownERL=true)  //
> must
> > > wait
> > > >> > for
> > > >> > > at least last known ELR
> > > >> > >   } else {
> > > >> > > if (strategy == Aggressive)
> > > >> > >   choose the last known leader if that is available, or a
> > random
> > > >> > leader
> > > >> > > if not)
> > > >> > > else
> > > >> > >   wait for last known leader to get back
> > > >> > >   }
> > > >> > > }
> > > >> > >
> > > >> > > The idea is that the Aggressive strategy would kick in as soon
> as
> > we
> > > >> lost
> > > >> > > the leader and would pick a leader from whoever is available;
> but
> > > the
> > > >> > > Balanced will only kick in when ELR is empty and will wait for
> the
> > > >> > brokers
> > > >> > > that likely have most data to be available.
> > > >> > >
> > > >> > > On Tue, Oct 3, 2023 at 3:04 PM Colin McCabe  >
> > > >> wrote:
> > > >> > >
> > > >> > > > On Tue, Oct 3, 2023, at 10:49, Jun Rao wrote:
> > > >> > > > > Hi, Calvin,
> > > >> > > > >
> > > >> > > > > Thanks for the update KIP. A few more comments.
> > > >> > > > >
> > > >> > > > > 41. Why would a user choose the option t

Re: [DISCUSS] KIP-983: Full speed async processing during rebalance

2023-10-13 Thread David Jacot
Hi Erik,

Thanks for the KIP. I haven’t fully read the KIP yet but I agree with the
weaknesses that you point out in it. I will continue to read it.

For your information, we are working full speed on implementing KIP-848
while also changing the internal threading model of consumer. Those changes
are already extremely large so I would rather prefer to complete them
before adding more on top of them. Moreover, I think that this KIP should
build on top of KIP-848 now. Would you agree with this?


Best,
David

Le ven. 13 oct. 2023 à 20:44, Erik van Oosten 
a écrit :

> Thanks Philip,
>
> No worries, I am not in a hurry. Knowing this is not forgotten is enough
> for me. If there is anything I can do to help the process please let me
> know.
>
> Kind regards,
>  Erik.
>
>
> Op 13-10-2023 om 20:29 schreef Philip Nee:
> > Hi Erik,
> >
> > Sorry for the delay, I have not finished reviewing the KIP, but I also
> have
> > not forgotten about it!
> >
> > In general, KIP review process can be lengthy, so I think mailing list is
> > the best bet to get the committer's attention.
> >
> > P
> >
> > On Fri, Oct 13, 2023 at 10:55 AM Erik van Oosten
> >  wrote:
> >
> >> Hi client developers,
> >>
> >> The text is updated so that it is more clear that you can only use
> >> auto-commit when doing synchronous processing (approach 1). I am
> >> assuming that auto-commit commits whatever was consumed in the previous
> >> poll.
> >>
> >> I am wondering why this KIP doesn't get more attention. Is async
> >> processing not something that the kafka client wants to support?
> >>
> >> Kind regards,
> >>   Erik.
> >>
> >>
> >> Op 25-09-2023 om 18:17 schreef Erik van Oosten:
> >>> Hi Viktor,
> >>>
> >>> Good questions!
> >>>
> >>> 1. Auto-commits would only work with approach 1 in the KIP. Any async
> >>> solution is incompatible with auto-commits. Do you think the text will
> >>> improve when this is mentioned?
> >>>
> >>> 2. That is entirely correct. If you use async commits you can await
> >>> completion by doing a single sync commit with an empty offsets Map
> >>> (this will work as of Kafka 3.6.0).
> >>>
> >>> Is there anything I can do to make the text clearer?
> >>>
> >>> Kind regards,
> >>>  Erik.
> >>>
> >>>
> >>> Op 25-09-2023 om 17:04 schreef Viktor Somogyi-Vass:
>  Hi Erik,
> 
>  I'm still trying to wrap my head around the KIP, however I have a few
>  questions that weren't clear to me regarding offset commits:
>  1. Would auto-commits interfere with the behavior defined in your KIP
> or
>  would it work the same as manual commits?
>  2. As I see you don't separate offset commits by whether they're sync
> or
>  async. For sync commits timing isn't really a problem but how would
> you
>  change work in case of async offset commits? There can be a few
> caveats
>  there as you may not know whether a commit is finished or not until
> your
>  callback is called.
> 
>  Thanks,
>  Viktor
> 
>  On Sat, Sep 23, 2023 at 4:00 PM Erik van Oosten
>   wrote:
> 
> > Hi all,
> >
> > I would like to start the discussion on KIP-983: Full speed async
> > processing during rebalance [1].
> >
> > The idea is that we can prevent the drop in throughput during a
> > cooperative rebalance.
> >
> > I am curious to your ideas and comments.
> >
> > Kind regards,
> >Erik.
> >
> > [1]
> >
> >
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-983%3A+Full+speed+async+processing+during+rebalance
> >> --
> >> Erik van Oosten
> >> e.vanoos...@grons.nl
> >> https://day-to-day-stuff.blogspot.com
> >>
> >>
> --
> Erik van Oosten
> e.vanoos...@grons.nl
> https://day-to-day-stuff.blogspot.com
>
>


Re: [DISCUSS] KIP-983: Full speed async processing during rebalance

2023-10-15 Thread David Jacot
Hi Erik,

Our current target is to have a preview in 3.7. This is subject to change
though.

Best,
David

Le dim. 15 oct. 2023 à 13:02, Erik van Oosten 
a écrit :

> Thanks Philip,
>
> That sounds pretty good. Meanwhile I'll continue to study KIP-848. It is
> a bit too much to digest in 1 go.
>
> Do you have a rough timeline for when the new consumer implementation
> can be tried out in non-production environments?
>
> Kind regards,
>  Erik.
>
>
> Op 14-10-2023 om 20:48 schreef Philip Nee:
> > Hi Erik,
> >
> > Thanks for the KIP, again.  I am also very much interested in the idea of
> > this KIP, and I want to let you know that we are rewriting the kafka
> > consumer using an event-driven approach, so I think the new impl would
> make
> > this KIP much easier to implement.  In a nutshell, the network IO will
> > become completely asynchronous to the application thread, so that the
> > blocking APIs won't stale the network send/receive.  In the new impl, the
> > main role of poll are 1. check if there are any background events such as
> > error or callback invocation, 2. notify the background that the user is
> > polling, and 3. check if there is any data to return to the user.
> > Because the background thread and application thread are inherently
> working
> > in an async fashion, it is possible to continue to process and commit
> > during the revocation period; however, we have to be very careful with
> the
> > state of partition ownership as you mentioned in the KIP.
> >
> > Please keep an eye out on the new consumer implementation, in case if you
> > are interested in digging in, it is the PrototypeKafkaConsumer module.
> It
> > is not fully functional but we are working full speed to get this to a
> good
> > state.
> >
> > Thanks - I am still reading to KIP and your previous KIP to see if I can
> > make more constructive suggestions here.
> > P
> >
> > On Fri, Oct 13, 2023 at 11:54 PM Erik van Oosten
> >  wrote:
> >
> >> Hello David,
> >>
> >> Thanks, I am happy to hear we agree on the problem. All the tiny details
> >> of an implementation are less important.
> >>
> >> I will read KIP-848 first to answer you question about its relation with
> >> KIP-983. But for sure it makes sense to complete the implementation of
> >> KIP-848 first.
> >>
> >> Kind regards,
> >>   Erik.
> >>
> >>
> >> Op 13-10-2023 om 21:00 schreef David Jacot:
> >>> Hi Erik,
> >>>
> >>> Thanks for the KIP. I haven’t fully read the KIP yet but I agree with
> the
> >>> weaknesses that you point out in it. I will continue to read it.
> >>>
> >>> For your information, we are working full speed on implementing KIP-848
> >>> while also changing the internal threading model of consumer. Those
> >> changes
> >>> are already extremely large so I would rather prefer to complete them
> >>> before adding more on top of them. Moreover, I think that this KIP
> should
> >>> build on top of KIP-848 now. Would you agree with this?
> >>>
> >>>
> >>> Best,
> >>> David
> >>>
> >>> Le ven. 13 oct. 2023 à 20:44, Erik van Oosten >> .invalid>
> >>> a écrit :
> >>>
> >>>> Thanks Philip,
> >>>>
> >>>> No worries, I am not in a hurry. Knowing this is not forgotten is
> enough
> >>>> for me. If there is anything I can do to help the process please let
> me
> >>>> know.
> >>>>
> >>>> Kind regards,
> >>>>Erik.
> >>>>
> >>>>
> >>>> Op 13-10-2023 om 20:29 schreef Philip Nee:
> >>>>> Hi Erik,
> >>>>>
> >>>>> Sorry for the delay, I have not finished reviewing the KIP, but I
> also
> >>>> have
> >>>>> not forgotten about it!
> >>>>>
> >>>>> In general, KIP review process can be lengthy, so I think mailing
> list
> >> is
> >>>>> the best bet to get the committer's attention.
> >>>>>
> >>>>> P
> >>>>>
> >>>>> On Fri, Oct 13, 2023 at 10:55 AM Erik van Oosten
> >>>>>   wrote:
> >>>>>
> >>>>>> Hi client developers,
> >>>>>>
> >>>>>> The text is updated s

Re: [VOTE] KIP-975: Docker Image for Apache Kafka

2023-10-29 Thread David Jacot
+1 (binding). Thanks for doing this!

Le dim. 29 oct. 2023 à 07:54, Manikumar  a
écrit :

> Hi,
>
> Thanks for the KIP.
>
> +1 (binding)
>
> Thanks
>
> On Fri, Oct 27, 2023 at 9:41 PM Ismael Juma  wrote:
>
> > Thanks for the KIP Krishna - looking forward to this. +1 (binding).
> >
> > On Thu, Oct 26, 2023 at 9:36 PM Krishna Agarwal <
> > krishna0608agar...@gmail.com> wrote:
> >
> > > Hi,
> > > I'd like to call a vote on KIP-975 which aims to publish an official
> > docker
> > > image for Apache Kafka.
> > >
> > > KIP -
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-975%3A+Docker+Image+for+Apache+Kafka
> > >
> > > Discussion thread -
> > > https://lists.apache.org/thread/3g43hps2dmkyxgglplrlwpsf7vkywkyy
> > >
> > > Regards,
> > > Krishna
> > >
> >
>


Re: Apache Kafka 3.7.0 Release

2023-11-02 Thread David Jacot
+1 from me as well. Thanks, Stan!

David

On Thu, Nov 2, 2023 at 6:04 PM Ismael Juma  wrote:

> Thanks Stanislav, +1
>
> Ismael
>
> On Thu, Nov 2, 2023 at 7:01 AM Stanislav Kozlovski
>  wrote:
>
> > Hi all,
> >
> > Given the discussion here and the lack of any pushback, I have changed
> the
> > dates of the release:
> > - KIP Freeze - *November 22 *(moved 4 days later)
> > - Feature Freeze - *December 6 *(moved 2 days earlier)
> > - Code Freeze - *December 20*
> >
> > If anyone has any thoughts against this proposal - please let me know! It
> > would be good to settle on this early. These will be the dates we're
> going
> > with
> >
> > Best,
> > Stanislav
> >
> > On Thu, Oct 26, 2023 at 12:15 AM Sophie Blee-Goldman <
> > sop...@responsive.dev>
> > wrote:
> >
> > > Thanks for the response and explanations -- I think the main question
> for
> > > me
> > > was whether we intended to permanently increase the KF -- FF gap from
> the
> > > historical 1 week to 3 weeks? Maybe this was a conscious decision and I
> > > just
> > >  missed the memo, hopefully someone else can chime in here. I'm all for
> > > additional though. And looking around at some of the recent releases,
> it
> > > seems like we haven't been consistently following the "usual" schedule
> > > since
> > > the 2.x releases.
> > >
> > > Anyways, my main concern was making sure to leave a full 2 weeks
> between
> > > feature freeze and code freeze, so I'm generally happy with the new
> > > proposal.
> > > Although I would still prefer to have the KIP freeze fall on a
> Wednesday
> > --
> > > Ismael actually brought up the same thing during the 3.5.0 release
> > > planning,
> > > so I'll just refer to his explanation for this:
> > >
> > > We typically choose a Wednesday for the various freeze dates - there
> are
> > > > often 1-2 day slips and it's better if that doesn't require people
> > > > working through the weekend.
> > > >
> > >
> > > (From this mailing list thread
> > > )
> > >
> > > Thanks for driving the release!
> > > Sophie
> > >
> > > On Wed, Oct 25, 2023 at 8:13 AM Stanislav Kozlovski
> > >  wrote:
> > >
> > > > Thanks for the thorough response, Sophie.
> > > >
> > > > - Added to the "Future Release Plan"
> > > >
> > > > > 1. Why is the KIP freeze deadline on a Saturday?
> > > >
> > > > It was simply added as a starting point - around 30 days from the
> > > > announcement. We can move it earlier to the 15th of November, but my
> > > > thinking is later is better with these things - it's already
> aggressive
> > > > enough. e.g given the choice of Nov 15 vs Nov 18, I don't necessarily
> > > see a
> > > > strong reason to choose 15.
> > > >
> > > > If people feel strongly about this, to make up for this, we can eat
> > into
> > > > the KF-FF time as I'll touch upon later, and move FF a few days
> earlier
> > > to
> > > > land on a Wednesday.
> > > >
> > > > This reduces the time one has to get their feature complete after KF,
> > but
> > > > allows for longer time to a KIP accepted, so the KF-FF gap can be
> made
> > up
> > > > when developing the feature in parallel.
> > > >
> > > > > , this makes it easy for everyone to remember when the next
> deadline
> > is
> > > > so they can make sure to get everything in on time. I worry that
> > varying
> > > > this will catch people off guard.
> > > >
> > > > I don't see much value in optimizing the dates for ease of memory -
> > > besides
> > > > the KIP Freeze (which is the base date), there are only two more
> dates
> > to
> > > > remember that are on the wiki. More importantly, we have a plethora
> of
> > > > tools that can be used to set up reminders - so a contributor doesn't
> > > > necessarily need to remember anything if they're serious about
> getting
> > > > their feature in.
> > > >
> > > > > 3. Is there a particular reason for having the feature freeze
> almost
> > a
> > > > full 3 weeks from the KIP freeze? ... having 3 weeks between the KIP
> > and
> > > > feature freeze (which are
> > > > usually separated by just a single week)?
> > > >
> > > > I was going off the last two releases, which had *20 days* (~3 weeks)
> > in
> > > > between KF & FF. Here are their dates:
> > > >
> > > > - AK 3.5
> > > >   - KF: 22 March
> > > >   - FF: 12 April
> > > > - (20 days after)
> > > >   - CF: 26 April
> > > > - (14 days after)
> > > >   - Release: 15 June
> > > >  - 50 days after CF
> > > > - AK 3.6
> > > >   - KF: 26 July
> > > >   - FF: 16 Aug
> > > > - (20 days after)
> > > >   - CF: 30 Aug
> > > > - (14 days after)
> > > >   - Release: 11 October
> > > > - 42 days after CF
> > > >
> > > > I don't know the precise reasoning for extending the time, nor what
> is
> > > the
> > > > most appropriate time - but having talked offline to some folks prior
> > to
> > > > this discussion, it seemed reasonable.
> > > >
> > > > Your proposal uses an aggressive 1-week gap between both, which is
> > quite
> > > >

[DISCUSS] Should we continue to merge without a green build? No!

2023-11-11 Thread David Jacot
Hi all,

The state of our CI worries me a lot. Just this week, we merged two PRs
with compilation errors and one PR introducing persistent failures. This
really hurts the quality and the velocity of the project and it basically
defeats the purpose of having a CI because we tend to ignore it nowadays.

Should we continue to merge without a green build? No! We should not so I
propose to prevent merging a pull request without a green build. This is a
really simple and bold move that will prevent us from introducing
regressions and will improve the overall health of the project. At the same
time, I think that we should disable all the known flaky tests, raise jiras
for them, find an owner for each of them, and fix them.

What do you think?

Best,
David


Re: [VOTE] KIP-1001; CurrentControllerId Metric

2023-11-21 Thread David Jacot
+1 from me.

Thanks,
David

On Mon, Nov 20, 2023 at 10:48 PM Jason Gustafson 
wrote:

> The KIP makes sense. +1
>
> On Mon, Nov 20, 2023 at 12:37 PM David Arthur
>  wrote:
>
> > Thanks Colin,
> >
> > +1 from me
> >
> > -David
> >
> > On Tue, Nov 14, 2023 at 3:53 PM Colin McCabe  wrote:
> >
> > > Hi all,
> > >
> > > I'd like to call a vote for KIP-1001: Add CurrentControllerId metric.
> > >
> > > Take a look here:
> > > https://cwiki.apache.org/confluence/x/egyZE
> > >
> > > best,
> > > Colin
> > >
> >
> >
> > --
> > -David
> >
>


Re: [DISCUSS] Should we continue to merge without a green build? No!

2023-11-22 Thread David Jacot
Hi all,

Thanks for the good discussion and all the comments. Overall, it seems that
we all agree on the bad state of our CI. That's a good first step!

I have talked to a few folks this week about it and it seems that many
folks (including me) are not comfortable with merging PRs at the moment
because the results of our builds are so bad. I had 40+ failed tests in one
of my PRs, all unrelated to my changes. It is really hard to be productive
with this.

Personally, I really want to move towards requiring a green build to merge
to trunk because this is a clear and binary signal. I agree that we need to
stabilize the builds before we could even require this so here is my
proposal.

1) We could leverage the `reports.junitXml.mergeReruns` option in gradle.
>From the doc [1]:

> When mergeReruns is enabled, if a test fails but is then retried and
succeeds, its failures will be recorded as  instead of
, within one . This is effectively the reporting
produced by the surefire plugin of Apache Maven™ when enabling reruns. If
your CI server understands this format, it will indicate that the test was
flaky. If it > does not, it will indicate that the test succeeded as it
will ignore the  information. If the test does not succeed
(i.e. it fails for every retry), it will be indicated as having failed
whether your tool understands this format or not.
> When mergeReruns is disabled (the default), each execution of a test will
be listed as a separate test case.

It would not resolve all the faky tests for sure but it would at least
reduce the noise. I see this as a means to get to green builds faster. I
played a bit with this setting and I discovered [2]. I was hoping that [3]
could help to resolve it but I need to confirm.

2) I suppose that we would still have flaky tests preventing us from
getting a green build even with the setting in place. For those, I think
that we need to review them one by one and decide whether we want to fix or
disable them. This is a short term effort to help us get to green builds.

3) When we get to a point where we can get green builds consistently, we
could enforce it.

4) Flaky tests won't disappear with this. They are just hidden. Therefore,
we also need a process to review the flaky tests and address them. Here, I
think that we could leverage the dashboard shared by Ismael. One
possibility would be to review it regularly and decide for each test
whether it should be fixed, disabled or even removed.

Please let me know what you think.

Another angle that we could consider is improving the CI infrastructure as
well. I think that many of those flaky tests are due to overloaded Jenkins
workers. We should perhaps discuss with the infra team to see whether we
could do something there.

Best,
David

[1] https://docs.gradle.org/current/userguide/java_testing.html#mergereruns
[2] https://github.com/gradle/gradle/issues/23324
[3] https://github.com/apache/kafka/pull/14687


On Wed, Nov 22, 2023 at 4:10 AM Ismael Juma  wrote:

> Hi,
>
> We have a dashboard already:
>
> [image: image.png]
>
>
> https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=America%2FLos_Angeles&search.values=trunk&tests.sortField=FLAKY
>
> On Tue, Nov 14, 2023 at 10:41 PM Николай Ижиков 
> wrote:
>
>> Hello guys.
>>
>> I want to tell you about one more approach to deal with flaky tests.
>> We adopt this approach in Apache Ignite community, so may be it can be
>> helpful for Kafka, also.
>>
>> TL;DR: Apache Ignite community have a tool that provide a statistic of
>> tests and can tell if PR introduces new failures.
>>
>> Apache Ignite has a many tests.
>> Latest «Run All» contains around 75k.
>> Most of test has integration style therefore count of flacky are
>> significant.
>>
>> We build a tool - Team City Bot [1]
>> That provides a combined statistic of flaky tests [2]
>>
>> This tool can compare results of Run All for PR and master.
>> If all OK one can comment jira ticket with a visa from bot [3]
>>
>> Visa is a quality proof of PR for Ignite committers.
>> And we can sort out most flaky tests and prioritize fixes with the bot
>> statistic [2]
>>
>> TC bot integrated with the Team City only, for now.
>> But, if Kafka community interested we can try to integrate it with
>> Jenkins.
>>
>> [1] https://github.com/apache/ignite-teamcity-bot
>> [2] https://tcbot2.sbt-ignite-dev.ru/current.html?branch=master&count=10
>> [3]
>> https://issues.apache.org/jira/browse/IGNITE-19950?focusedCommentId=17767394&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17767394
>>
>>
>>
>> > 15 нояб. 2023 г., в 09:18, Ismael Juma  написал(а):
>> >
>> > To use the pain analogy, people seem to have really good painkillers and
>> > hence they somehow don't feel the pain already. ;)
>> >
>> > The reality is that important and high quality tests will get fixed.
>> Poor
>> > quality tests (low signal to noise ratio) might not get fixed a

Re: [DISCUSS] Should we continue to merge without a green build? No!

2023-11-22 Thread David Jacot
Hi Ismael,

No, I was not aware of KAFKA-12216. My understanding is that we could still
do it without the JUnitFlakyTestDataPublisher plugin and we could use
gradle enterprise for this. Or do you think that reporting the flaky tests
in the build results is required?

David

On Wed, Nov 22, 2023 at 9:35 AM Ismael Juma  wrote:

> Hi David,
>
> Did you take a look at https://issues.apache.org/jira/browse/KAFKA-12216?
> I
> looked into this option already (yes, there isn't much that we haven't
> considered in this space).
>
> Ismael
>
> On Wed, Nov 22, 2023 at 12:24 AM David Jacot 
> wrote:
>
> > Hi all,
> >
> > Thanks for the good discussion and all the comments. Overall, it seems
> that
> > we all agree on the bad state of our CI. That's a good first step!
> >
> > I have talked to a few folks this week about it and it seems that many
> > folks (including me) are not comfortable with merging PRs at the moment
> > because the results of our builds are so bad. I had 40+ failed tests in
> one
> > of my PRs, all unrelated to my changes. It is really hard to be
> productive
> > with this.
> >
> > Personally, I really want to move towards requiring a green build to
> merge
> > to trunk because this is a clear and binary signal. I agree that we need
> to
> > stabilize the builds before we could even require this so here is my
> > proposal.
> >
> > 1) We could leverage the `reports.junitXml.mergeReruns` option in gradle.
> > From the doc [1]:
> >
> > > When mergeReruns is enabled, if a test fails but is then retried and
> > succeeds, its failures will be recorded as  instead of
> > , within one . This is effectively the reporting
> > produced by the surefire plugin of Apache Maven™ when enabling reruns. If
> > your CI server understands this format, it will indicate that the test
> was
> > flaky. If it > does not, it will indicate that the test succeeded as it
> > will ignore the  information. If the test does not succeed
> > (i.e. it fails for every retry), it will be indicated as having failed
> > whether your tool understands this format or not.
> > > When mergeReruns is disabled (the default), each execution of a test
> will
> > be listed as a separate test case.
> >
> > It would not resolve all the faky tests for sure but it would at least
> > reduce the noise. I see this as a means to get to green builds faster. I
> > played a bit with this setting and I discovered [2]. I was hoping that
> [3]
> > could help to resolve it but I need to confirm.
> >
> > 2) I suppose that we would still have flaky tests preventing us from
> > getting a green build even with the setting in place. For those, I think
> > that we need to review them one by one and decide whether we want to fix
> or
> > disable them. This is a short term effort to help us get to green builds.
> >
> > 3) When we get to a point where we can get green builds consistently, we
> > could enforce it.
> >
> > 4) Flaky tests won't disappear with this. They are just hidden.
> Therefore,
> > we also need a process to review the flaky tests and address them. Here,
> I
> > think that we could leverage the dashboard shared by Ismael. One
> > possibility would be to review it regularly and decide for each test
> > whether it should be fixed, disabled or even removed.
> >
> > Please let me know what you think.
> >
> > Another angle that we could consider is improving the CI infrastructure
> as
> > well. I think that many of those flaky tests are due to overloaded
> Jenkins
> > workers. We should perhaps discuss with the infra team to see whether we
> > could do something there.
> >
> > Best,
> > David
> >
> > [1]
> > https://docs.gradle.org/current/userguide/java_testing.html#mergereruns
> > [2] https://github.com/gradle/gradle/issues/23324
> > [3] https://github.com/apache/kafka/pull/14687
> >
> >
> > On Wed, Nov 22, 2023 at 4:10 AM Ismael Juma  wrote:
> >
> > > Hi,
> > >
> > > We have a dashboard already:
> > >
> > > [image: image.png]
> > >
> > >
> > >
> >
> https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=America%2FLos_Angeles&search.values=trunk&tests.sortField=FLAKY
> > >
> > > On Tue, Nov 14, 2023 at 10:41 PM Николай Ижиков 
> > > wrote:
> > >
> > >> Hello guys.
> > >>
> > >> I want to tell you about

Re: [DISCUSS] Should we continue to merge without a green build? No!

2023-11-27 Thread David Jacot
Hi all,

I am still experimenting with reducing the noise of flaky tests in build
results. I should have results to share early next week.

Chris, I am also for a programmatic gate. Regarding using ignoreFailures,
it seems risky because the build may be green but with failed tests, no?

I would also like to make it clear that the current rule applies until we
agree on a way forward here. At minimum, I think that a build should be
yellow for all the combinations and the failed tests should have been
triaged to ensure that they are not related to the changes. We should not
merge when a build is red or has not completed.

Best,
David

On Sat, Nov 25, 2023 at 5:25 AM Chris Egerton 
wrote:

> Hi all,
>
> There's a lot to catch up on here but I wanted to clarify something.
> Regarding this comment from Sophie:
>
>
> > Yet multiple people in this thread so
> far have voiced support for "gating merges on the successful completion of
> all parts of the build except tests". Just to be totally clear, I really
> don't think that was ever in question -- though it certainly doesn't hurt
> to remind everyone.
>
> > So, this thread is not about whether or not to merge with failing
> *builds, *but it's
> whether it should be acceptable to merge with failing *tests.*
>
>
> I think there's a misunderstanding here. I was suggesting
> programmatic gating, not manual. If we could disable these types of changes
> from being merged, instead of relying on committers to check and interpret
> Jenkins results, that'd be a quick win IMO. And, because of the
> already-discussed issues with flaky tests, it seemed difficult to disable
> PRs from being merged with failing tests--just for other parts of the
> build.
>
> However, I think the retry logic brought up by David could be sufficient to
> skip that kind of intermediate step and allow us to just start
> programmatically disabling PR merges if the build (including) tests fails.
> But if anyone's interested, we can still prevent failing tests from failing
> the build with the ignoreFailures property [1].
>
> [1] -
>
> https://docs.gradle.org/current/dsl/org.gradle.api.tasks.testing.Test.html#org.gradle.api.tasks.testing.Test:ignoreFailures
>
> Cheers,
>
> Chris
>
> On Wed, Nov 22, 2023 at 3:00 AM Ismael Juma  wrote:
>
> > I think it breaks the Jenkins output otherwise. Feel free to test it via
> a
> > PR.
> >
> > Ismael
> >
> > On Wed, Nov 22, 2023, 12:42 AM David Jacot 
> > wrote:
> >
> > > Hi Ismael,
> > >
> > > No, I was not aware of KAFKA-12216. My understanding is that we could
> > still
> > > do it without the JUnitFlakyTestDataPublisher plugin and we could use
> > > gradle enterprise for this. Or do you think that reporting the flaky
> > tests
> > > in the build results is required?
> > >
> > > David
> > >
> > > On Wed, Nov 22, 2023 at 9:35 AM Ismael Juma  wrote:
> > >
> > > > Hi David,
> > > >
> > > > Did you take a look at
> > https://issues.apache.org/jira/browse/KAFKA-12216
> > > ?
> > > > I
> > > > looked into this option already (yes, there isn't much that we
> haven't
> > > > considered in this space).
> > > >
> > > > Ismael
> > > >
> > > > On Wed, Nov 22, 2023 at 12:24 AM David Jacot
> >  > > >
> > > > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > Thanks for the good discussion and all the comments. Overall, it
> > seems
> > > > that
> > > > > we all agree on the bad state of our CI. That's a good first step!
> > > > >
> > > > > I have talked to a few folks this week about it and it seems that
> > many
> > > > > folks (including me) are not comfortable with merging PRs at the
> > moment
> > > > > because the results of our builds are so bad. I had 40+ failed
> tests
> > in
> > > > one
> > > > > of my PRs, all unrelated to my changes. It is really hard to be
> > > > productive
> > > > > with this.
> > > > >
> > > > > Personally, I really want to move towards requiring a green build
> to
> > > > merge
> > > > > to trunk because this is a clear and binary signal. I agree that we
> > > need
> > > > to
> > > > > stabilize the builds before we could even require this so here is
> my
> > > > > proposal.
> > > > >

[DISCUSS] KIP-1041: Drop `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8)

2024-05-02 Thread David Jacot
Hi folks,

I have put together a very small KIP to
deprecate offsets.commit.required.acks in 3.8 and remove it in 4.0. See the
motivation for the reason.

KIP: https://cwiki.apache.org/confluence/x/9YobEg

Please let me know what you think.

Best,
David


[VOTE] KIP-1041: Drop `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8)

2024-05-08 Thread David Jacot
Hi folks,

I'd like to start a voting thread for KIP-1041: Drop
`offsets.commit.required.acks` config in 4.0 (deprecate in 3.8).

KIP: https://cwiki.apache.org/confluence/x/9YobEg

Best,
David


Re: [VOTE] KIP-1041: Drop `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8)

2024-05-13 Thread David Jacot
+1 (binding) from me too.

The KIP passes with binding votes from Justine, Manikumar and me; and
non-binding votes from Andrew and Federico.

Thanks,
David

On Mon, May 13, 2024 at 1:52 PM Manikumar  wrote:

> +1 (binding).
>
> Thanks for the KIP.
>
> Manikumar
>
> On Wed, May 8, 2024 at 9:55 PM Justine Olshan
>  wrote:
> >
> > +1 (binding)
> >
> > Thanks,
> > Justine
> >
> > On Wed, May 8, 2024 at 8:36 AM Federico Valeri 
> wrote:
> >
> > > +1 non binding
> > >
> > > Thanks
> > >
> > > On Wed, May 8, 2024 at 5:27 PM Andrew Schofield
> > >  wrote:
> > > >
> > > > Hi,
> > > > Thanks for the KIP.
> > > >
> > > > +1 (non-binding)
> > > >
> > > > Thanks,
> > > > Andrew
> > > >
> > > > > On 8 May 2024, at 15:48, David Jacot 
> > > wrote:
> > > > >
> > > > > Hi folks,
> > > > >
> > > > > I'd like to start a voting thread for KIP-1041: Drop
> > > > > `offsets.commit.required.acks` config in 4.0 (deprecate in 3.8).
> > > > >
> > > > > KIP: https://cwiki.apache.org/confluence/x/9YobEg
> > > > >
> > > > > Best,
> > > > > David
> > > >
> > >
>


Re: [VOTE] KIP-932: Queues for Kafka

2024-05-16 Thread David Jacot
Hi Andrew,

Thanks for the KIP! This is really exciting! +1 (binding) from me.

One note regarding the partition assignor interface changes that you
proposed, it would be great to get the changes in 3.8 in order to not break
the API of KIP-848 after the preview.

Best,
David

On Wed, May 15, 2024 at 10:37 PM Jun Rao  wrote:

> Hi, Andrew,
>
> Thanks for the update. Should we mark whether those metrics are
> standard/required for KIP-714?
>
> Jun
>
> On Tue, May 14, 2024 at 7:31 AM Andrew Schofield <
> andrew_schofi...@live.com>
> wrote:
>
> > Hi,
> > I have made a small update to the KIP as a result of testing the new
> > share consumer with client telemetry (KIP-714).
> >
> > I’ve added telemetry metric names to the table of client metrics and
> > also updated the metric group names so that the resulting client metrics
> > sent to the broker have consistent names.
> >
> > Thanks,
> > Andrew
> >
> > > On 8 May 2024, at 12:51, Manikumar  wrote:
> > >
> > > Hi Andrew,
> > >
> > > Thanks for the KIP.  Great write-up!
> > >
> > > +1 (binding)
> > >
> > > Thanks,
> > >
> > > On Wed, May 8, 2024 at 12:17 PM Satish Duggana <
> satish.dugg...@gmail.com>
> > wrote:
> > >>
> > >> Hi Andrew,
> > >> Thanks for the nice KIP, it will allow other messaging use cases to be
> > >> onboarded to Kafka.
> > >>
> > >> +1 from me.
> > >>
> > >> Satish.
> > >>
> > >> On Tue, 7 May 2024 at 03:41, Jun Rao 
> wrote:
> > >>>
> > >>> Hi, Andrew,
> > >>>
> > >>> Thanks for the KIP. +1
> > >>>
> > >>> Jun
> > >>>
> > >>> On Mon, Mar 18, 2024 at 11:00 AM Edoardo Comar <
> edoardli...@gmail.com>
> > >>> wrote:
> > >>>
> >  Thanks Andrew,
> > 
> >  +1 (binding)
> > 
> >  Edo
> > 
> >  On Mon, 18 Mar 2024 at 16:32, Kenneth Eversole
> >   wrote:
> > >
> > > Hi Andrew
> > >
> > > + 1 (Non-Binding)
> > >
> > > This will be great addition to Kafka
> > >
> > > On Mon, Mar 18, 2024 at 8:27 AM Apoorv Mittal <
> > apoorvmitta...@gmail.com>
> > > wrote:
> > >
> > >> Hi Andrew,
> > >> Thanks for writing the KIP. This is indeed going to be a valuable
> >  addition
> > >> to the Kafka, excited to see the KIP.
> > >>
> > >> + 1 (Non-Binding)
> > >>
> > >> Regards,
> > >> Apoorv Mittal
> > >> +44 7721681581
> > >>
> > >>
> > >> On Sun, Mar 17, 2024 at 11:16 PM Andrew Schofield <
> > >> andrew_schofield_j...@outlook.com> wrote:
> > >>
> > >>> Hi,
> > >>> I’ve been working to complete KIP-932 over the past few months
> and
> > >>> discussions have quietened down.
> > >>>
> > >>> I’d like to open the voting for KIP-932:
> > >>>
> > >>>
> > >>>
> > >>
> > 
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-932%3A+Queues+for+Kafka
> > >>>
> > >>> Thanks,
> > >>> Andrew
> > >>
> > 
> >
> >
>


Re: [VOTE] KIP-950: Tiered Storage Disablement

2024-05-30 Thread David Jacot
Hi all,

Thanks for the KIP. This is definitely a worthwhile feature. However, I am
a bit sceptical on the ZK part of the story. The 3.8 release is supposed to
be the last one supporting ZK so I don't really see how we could bring it
to ZK, knowing that we don't plan to do a 3.9 release (current plan). I
strongly suggest clarifying this before implementing the ZK part in order
to avoid having new code [1] being deleted right after 3.8 is released
:). Personally, I agree with Chia-Ping and Mickael. We could drop the ZK
part.

[1] https://github.com/apache/kafka/pull/16131

Best,
David

On Tue, May 28, 2024 at 1:31 PM Mickael Maison 
wrote:

> Hi,
>
> I agree with Chia-Ping, I think we could drop the ZK variant
> altogether, especially if this is not going to make it in 3.8.0.
> Even if we end up needing a 3.9.0 release, I wouldn't write a bunch of
> new ZooKeeper-related code in that release to delete it all right
> after in 4.0.
>
> Thanks,
> Mickael
>
> On Fri, May 24, 2024 at 5:03 PM Christo Lolov 
> wrote:
> >
> > Hello!
> >
> > I am closing this vote as ACCEPTED with 3 binding +1 (Luke, Chia-Ping and
> > Satish) and 1 non-binding +1 (Kamal) - thank you for the reviews!
> >
> > Realistically, I don't think I have the bandwidth to get this in 3.8.0.
> > Due to this, I will mark tentatively the Zookeeper part for 3.9 if the
> > community decides that they do in fact want one more 3.x release.
> > I will mark the KRaft part as ready to be started and aiming for either
> 4.0
> > or 3.9.
> >
> > Best,
> > Christo
>


Re: [DISCUSS] Apache Kafka 3.8.0 release

2024-06-14 Thread David Jacot
The plan sounds good to me. I suppose that we will follow our regular
cadence for 4.0 and release it four months after 3.9 (in November?). Is
this correct?

Best,
David

Le ven. 14 juin 2024 à 21:57, José Armando García Sancio
 a écrit :

> +1 on the proposed release plan for 3.8.
>
> Thanks!
>
> On Fri, Jun 14, 2024 at 3:33 PM Ismael Juma  wrote:
> >
> > +1 to the plan we converged on in this thread.
> >
> > Ismael
> >
> > On Fri, Jun 14, 2024 at 10:46 AM Josep Prat  >
> > wrote:
> >
> > > Hi all,
> > >
> > > Thanks Colin, yes go ahead.
> > >
> > > As we are now past code freeze I would like to ask everyone involved
> in a
> > > KIP that is not yet complete, to verify if what landed on the 3.8
> branch
> > > needs to be reverted or if it can stay. Additionally, I'll be pinging
> KIPs
> > > and Jira reporters asking for their status as some Jiras seem to have
> all
> > > related GitHub PRs merged but their status is still Open or In
> Progress.
> > > I'll be checking all the open blockers and check if they are really a
> > > blocker or can be pushed.
> > >
> > >
> > > Regarding timeline, I'll attempt to generate the first RC on Wednesday
> or
> > > Thursday, so please revert any changes you deem necessary by then. If
> you
> > > need more time, please ping me.
> > >
> > > Best,
> > >
> > > -
> > > Josep Prat
> > > Open Source Engineering Director, Aiven
> > > josep.p...@aiven.io   |   +491715557497 | aiven.io
> > > Aiven Deutschland GmbH
> > > Alexanderufer 3-7, 10117 Berlin
> > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> > > Amtsgericht Charlottenburg, HRB 209739 B
> > >
> > > On Fri, Jun 14, 2024, 19:25 Colin McCabe  wrote:
> > >
> > > > Hi all,
> > > >
> > > > We have had many delays with releases this year. We should try to get
> > > back
> > > > on schedule.
> > > >
> > > > I agree with the idea that was proposed a few times in this thread of
> > > > drawing a line under 3.8 now, and doing a short 3.9 release. I
> posted a
> > > 3.9
> > > > release plan here:
> > > > https://cwiki.apache.org/confluence/display/KAFKA/Release+Plan+3.9.0
> > > >
> > > > I think we could start doing RCs for 3.8.0 as early as next week.
> There
> > > > are a few things that need to be reverted first (anything related to
> > > > KIP-853 or KIP-966).
> > > >
> > > > Josep, if you agree, I will update KIP-1012 to reflect that these
> things
> > > > are landing in 3.9 rather than 3.8. And we can start doing all the
> normal
> > > > release stuff. The main blocker JIRA I'm aware of is KAFKA-16946,
> which
> > > is
> > > > a very simple fix.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Fri, Jun 14, 2024, at 03:48, Satish Duggana wrote:
> > > > > +1 on going with 3.8 release with the existing plan and targeting
> the
> > > > > required features in 3.9 timelines. 4.0 will be targeted in the
> usual
> > > > > cycle(4 months) after 3.9 is released.
> > > > >
> > > > >
> > > > > On Fri, 14 Jun 2024 at 15:19, Edoardo Comar  >
> > > > wrote:
> > > > >>
> > > > >> Josep,
> > > > >> past the deadline sorry but I can't see reasons not to cherry-pick
> > > this
> > > > >> https://github.com/apache/kafka/pull/16326
> > > > >>
> > > > >> On Wed, 12 Jun 2024 at 17:14, Josep Prat
>  > > >
> > > > wrote:
> > > > >> >
> > > > >> > Hi Edoardo,
> > > > >> >
> > > > >> > Correct, you can still cherry-pick this one.
> > > > >> >
> > > > >> > I'll send an email tomorrow morning (CEST) asking maintainers to
> > > stop
> > > > >> > cherry picking commits unless we discuss it beforehand.
> > > > >> >
> > > > >> > Best,
> > > > >> >
> > > > >> > On Wed, Jun 12, 2024 at 6:09 PM Edoardo Comar <
> > > edoardli...@gmail.com>
> > > > wrote:
> > > > >> >
> > > > >> > > Hi Josep, I understand I am still in time to cherry-pick on
> 3.8.0
> > > > >> > > https://github.com/apache/kafka/pull/16229
> > > > >> > >
> > > > >> > > right?
> > > > >> > > thanks
> > > > >> > >
> > > > >> > > On Wed, 12 Jun 2024 at 11:34, Ivan Yurchenko 
> > > > wrote:
> > > > >> > > >
> > > > >> > > > Hi,
> > > > >> > > >
> > > > >> > > > I'll try to do all the fixes and changes for KIP-899 [1]
> sooner
> > > > today,
> > > > >> > > but please proceed with the release if I don't manage.
> > > > >> > > >
> > > > >> > > > Ivan
> > > > >> > > >
> > > > >> > > > [1] https://github.com/apache/kafka/pull/13277
> > > > >> > > >
> > > > >> > > > On Wed, Jun 12, 2024, at 12:54, Josep Prat wrote:
> > > > >> > > > > Hi Luke,
> > > > >> > > > > I think Jose, also mentioned that it won't be ready for
> v3.8.0
> > > > (but he
> > > > >> > > can
> > > > >> > > > > confirm this). My question now would be, given that it
> seems
> > > we
> > > > would
> > > > >> > > need
> > > > >> > > > > a v3.9.0, do you think it's important to include
> > > > >> > > > > https://github.com/apache/kafka/pull/16284 in v3.8.0?
> > > > >> > > > >
> > > > >> > > > > Best,
> > > > >> > > > >
> > > > >> > > > > On Wed, Jun 12, 2024 at 11:40 AM Luke Chen <
> show...@gmail.com
> > > >

Re: [DISCUSS] Apache Kafka 3.8.0 release

2024-06-14 Thread David Jacot
I agree that we should keep 4.0 time-based. My question is based on which
release. If I understand you, you would like to keep it based on 3.8. This
means that 4.0 would be released in October. It would be helpful to fix the
dates so we can plan accordingly. I will start a separate thread on Monday.

David

Le sam. 15 juin 2024 à 00:44, Colin McCabe  a écrit :

> +1. I think it would be good to keep 4.0 time-based. Most of the refactors
> we want to do are optional in some sense and can be descoped if time runs
> out. For example, we can drop support for JDK 8 without immediately
> refactoring everything that could benefit from the improvements in JDK9+.
>
> best,
> Colin
>
>
> On Fri, Jun 14, 2024, at 15:37, Matthias J. Sax wrote:
> > That's my understanding, and I would advocate strongly to keep the 4.0
> > release schedule as planed originally.
> >
> > The 3.9 one should really be an additional "out of schedule" release not
> > impacting any other releases.
> >
> >
> > -Matthias
> >
> > On 6/14/24 1:29 PM, David Jacot wrote:
> >> The plan sounds good to me. I suppose that we will follow our regular
> >> cadence for 4.0 and release it four months after 3.9 (in November?). Is
> >> this correct?
> >>
> >> Best,
> >> David
> >>
> >> Le ven. 14 juin 2024 à 21:57, José Armando García Sancio
> >>  a écrit :
> >>
> >>> +1 on the proposed release plan for 3.8.
> >>>
> >>> Thanks!
> >>>
> >>> On Fri, Jun 14, 2024 at 3:33 PM Ismael Juma  wrote:
> >>>>
> >>>> +1 to the plan we converged on in this thread.
> >>>>
> >>>> Ismael
> >>>>
> >>>> On Fri, Jun 14, 2024 at 10:46 AM Josep Prat
>  >>>>
> >>>> wrote:
> >>>>
> >>>>> Hi all,
> >>>>>
> >>>>> Thanks Colin, yes go ahead.
> >>>>>
> >>>>> As we are now past code freeze I would like to ask everyone involved
> >>> in a
> >>>>> KIP that is not yet complete, to verify if what landed on the 3.8
> >>> branch
> >>>>> needs to be reverted or if it can stay. Additionally, I'll be pinging
> >>> KIPs
> >>>>> and Jira reporters asking for their status as some Jiras seem to have
> >>> all
> >>>>> related GitHub PRs merged but their status is still Open or In
> >>> Progress.
> >>>>> I'll be checking all the open blockers and check if they are really a
> >>>>> blocker or can be pushed.
> >>>>>
> >>>>>
> >>>>> Regarding timeline, I'll attempt to generate the first RC on
> Wednesday
> >>> or
> >>>>> Thursday, so please revert any changes you deem necessary by then. If
> >>> you
> >>>>> need more time, please ping me.
> >>>>>
> >>>>> Best,
> >>>>>
> >>>>> -
> >>>>> Josep Prat
> >>>>> Open Source Engineering Director, Aiven
> >>>>> josep.p...@aiven.io   |   +491715557497 | aiven.io
> >>>>> Aiven Deutschland GmbH
> >>>>> Alexanderufer 3-7, 10117 Berlin
> >>>>> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> >>>>> Amtsgericht Charlottenburg, HRB 209739 B
> >>>>>
> >>>>> On Fri, Jun 14, 2024, 19:25 Colin McCabe  wrote:
> >>>>>
> >>>>>> Hi all,
> >>>>>>
> >>>>>> We have had many delays with releases this year. We should try to
> get
> >>>>> back
> >>>>>> on schedule.
> >>>>>>
> >>>>>> I agree with the idea that was proposed a few times in this thread
> of
> >>>>>> drawing a line under 3.8 now, and doing a short 3.9 release. I
> >>> posted a
> >>>>> 3.9
> >>>>>> release plan here:
> >>>>>>
> https://cwiki.apache.org/confluence/display/KAFKA/Release+Plan+3.9.0
> >>>>>>
> >>>>>> I think we could start doing RCs for 3.8.0 as early as next week.
> >>> There
> >>>>>> are a few things that need to be reverted first (anything related to
> >>>>>> KIP-853 or KIP-966).
> >>>>>>
> >>>>>> 

Re: [DISCUSS] Apache Kafka 3.9.0 release

2024-06-17 Thread David Jacot
+1 for the release plan. Thanks!

+1 for releasing 4.0 four months after 3.9. 4.0 is actually a pretty big
release as we will GA KIP-848, including new group coordinator and new
consumer rebalance protocol. This is a pretty big change :).

Best,
David

Le lun. 17 juin 2024 à 20:36, Colin McCabe  a écrit :

> Hi all,
>
> Thanks, everyone.
>
> Quick update: on the release plan page, I moved feature freeze forward and
> code freeze by one week to make sure we can hit that. No other dates
> changed.
>
> With regard to 4.0, I was assuming that we'd do it 4 months after 3.9 was
> released. I think for that release, we should front-load all the "remove
> deprecated stuff" work that has piled up, and try to really hit all the
> release milestone dates. I don't think 4.0 needs to be a very ambitious
> release, it really is mostly about the removals and the new JDK (which are
> a big deal, but I hope should fit within the normal timeframes). That being
> said, if people want a shorter 4.0, I'm open to that, as long as we're
> confident we could actually do that :)
>
> best,
> Colin
>
>
> On Fri, Jun 14, 2024, at 20:56, Sophie Blee-Goldman wrote:
> > +1, thank you Colin
> >
> > Given the July freeze deadlines, I take it we are going with the "short
> > 3.9.0 release" option and that the existence of this release will impact
> > the 4.0.0 deadlines which will still follow the usual schedule -- in
> other
> > words, this is an "additional release" outside of the regular timeline.
> Is
> > this understanding correct?
> >
> > On Fri, Jun 14, 2024 at 5:57 PM Chia-Ping Tsai 
> wrote:
> >
> >> +1 thanks Colin!
> >>
>


Re: [DISCUSS] Apache Kafka 3.8.0 release

2024-06-17 Thread David Jacot
I meant it from a time perspective, not from a branching point perspective.
Sorry for the confusion. As said in the other thread, doing it four months
after 3.9 is desirable for KIP-848 as I expect that we will need time to
stabilize everything after switching all the default configs once 3.9 is
cut.

David

Le lun. 17 juin 2024 à 19:33, Matthias J. Sax  a écrit :

> Why would 4.0 be based on 3.8? My understanding is, that it will be
> based on 3.9.
>
> -Matthias
>
> On 6/14/24 11:22 PM, David Jacot wrote:
> > I agree that we should keep 4.0 time-based. My question is based on which
> > release. If I understand you, you would like to keep it based on 3.8.
> This
> > means that 4.0 would be released in October. It would be helpful to fix
> the
> > dates so we can plan accordingly. I will start a separate thread on
> Monday.
> >
> > David
> >
> > Le sam. 15 juin 2024 à 00:44, Colin McCabe  a écrit
> :
> >
> >> +1. I think it would be good to keep 4.0 time-based. Most of the
> refactors
> >> we want to do are optional in some sense and can be descoped if time
> runs
> >> out. For example, we can drop support for JDK 8 without immediately
> >> refactoring everything that could benefit from the improvements in
> JDK9+.
> >>
> >> best,
> >> Colin
> >>
> >>
> >> On Fri, Jun 14, 2024, at 15:37, Matthias J. Sax wrote:
> >>> That's my understanding, and I would advocate strongly to keep the 4.0
> >>> release schedule as planed originally.
> >>>
> >>> The 3.9 one should really be an additional "out of schedule" release
> not
> >>> impacting any other releases.
> >>>
> >>>
> >>> -Matthias
> >>>
> >>> On 6/14/24 1:29 PM, David Jacot wrote:
> >>>> The plan sounds good to me. I suppose that we will follow our regular
> >>>> cadence for 4.0 and release it four months after 3.9 (in November?).
> Is
> >>>> this correct?
> >>>>
> >>>> Best,
> >>>> David
> >>>>
> >>>> Le ven. 14 juin 2024 à 21:57, José Armando García Sancio
> >>>>  a écrit :
> >>>>
> >>>>> +1 on the proposed release plan for 3.8.
> >>>>>
> >>>>> Thanks!
> >>>>>
> >>>>> On Fri, Jun 14, 2024 at 3:33 PM Ismael Juma 
> wrote:
> >>>>>>
> >>>>>> +1 to the plan we converged on in this thread.
> >>>>>>
> >>>>>> Ismael
> >>>>>>
> >>>>>> On Fri, Jun 14, 2024 at 10:46 AM Josep Prat
> >>  >>>>>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> Thanks Colin, yes go ahead.
> >>>>>>>
> >>>>>>> As we are now past code freeze I would like to ask everyone
> involved
> >>>>> in a
> >>>>>>> KIP that is not yet complete, to verify if what landed on the 3.8
> >>>>> branch
> >>>>>>> needs to be reverted or if it can stay. Additionally, I'll be
> pinging
> >>>>> KIPs
> >>>>>>> and Jira reporters asking for their status as some Jiras seem to
> have
> >>>>> all
> >>>>>>> related GitHub PRs merged but their status is still Open or In
> >>>>> Progress.
> >>>>>>> I'll be checking all the open blockers and check if they are
> really a
> >>>>>>> blocker or can be pushed.
> >>>>>>>
> >>>>>>>
> >>>>>>> Regarding timeline, I'll attempt to generate the first RC on
> >> Wednesday
> >>>>> or
> >>>>>>> Thursday, so please revert any changes
> <https://www.google.com/maps/search/,+so+please+revert+any+changes+?entry=gmail&source=g>you
> deem necessary by then. If
> >>>>> you
> >>>>>>> need more time, please ping me.
> >>>>>>>
> >>>>>>> Best,
> >>>>>>>
> >>>>>>> -
> >>>>>>> Josep Prat
> >>>>>>> Open Source Engineering Director, Aiven
> >>>>>>> josep.p...@aiven.io   |   +491715557497 | aiven.io
>

Re: [DISCUSS] KIP-1014: Managing Unstable Features in Apache Kafka

2024-06-27 Thread David Jacot
Hi all,

I think that it would be nice to have an official way to enable
non-production-ready features in order to have a way to test them in
development/soak clusters. For instance, I would like the new consumer
protocol to be disabled by default but users should be able to enable it if
they want to test it in their environment. Diverging a bit here but for
group.version=1, it should be the default in 4.0 and it should be usable in
3.8/3.9 by for instance setting it with `kafka-features`. I don't think
that we allow this with KIP-1022 unless the unstable/unrelease flag is set
because group.version=1 is attached to MV_4_0.

Best,
David

On Thu, Jun 27, 2024 at 12:50 AM Colin McCabe  wrote:

> Hi Jun,
>
> KIP-1014 is explicitly NOT for EA features, since EA features need to be
> usable by non-developers.
>
> I think it's important to be clear about this. Maybe "unreleased" would be
> a better name than "unstable" since people seem to have lots of different
> ideas about what "unstable" means, which are very different than the
> intention of this KIP. What do you think?
>
> For a developer making changes to the code, making an additional change to
> enable testing outside of JUnit should be fine. I think this is actually
> critical to making this work, since if we allow non-developers to use
> KIP-1014 features, we'll get bogged down in compatibility discussions (no
> matter what the KIP says).
>
> best,
> Colin
>
> On Wed, Jun 26, 2024, at 14:49, Jun Rao wrote:
> > Hi, Colin,
> >
> > Thanks for the reply.
> >
> > 4. "A developer could modify the code to allow unstable features outside
> of
> > JUnit, and then run whatever they want."
> > Hmm, it's inconvenient for a developer to make some temporary change just
> > to test an unstable feature outside of junit, right?
> >
> > Also, how does a user test an EA feature in a release? It's inconvenient
> > for a user to change code and recompile the binary.
> >
> > Jun
> >
> >
> > On Wed, Jun 26, 2024 at 1:38 PM Colin McCabe  wrote:
> >
> >> On Wed, Jun 26, 2024, at 13:16, Jun Rao wrote:
> >> > Hi, Colin,
> >> >
> >> > Thanks for the reply.
> >> >
> >> > 1.
> >>
> https://kafka.apache.org/protocol.html#The_Messages_ConsumerGroupDescribe
> >> > lists ConsumerGroupDescribeRequest, whose latest version is unstable.
> >> >
> >>
> >> Hi Jun,
> >>
> >> I think that is a bug.
> >>
> >> >
> >> > 4. "As devlopers, they can change the code to do this if they want."
> >> > Just to be clear. A developer could be able to test unstable MV/RPCs
> by
> >> > enabling unstable.features.enable in a real cluster, right?
> >>
> >> A developer could modify the code to allow unstable features outside of
> >> JUnit, and then run whatever they want.
> >>
> >> >
> >> > "But I think it's important that this should NOT work in our actual
> Kafka
> >> > releases"
> >> > Are you saying unstable MV/RPCs can't be enabled in Kafka releases
> with
> >> > unstable.features.enable set to true? How do we plan to enforce that?
> >> >
> >>
> >> We can just unset the configuration key in KafkaRaftServer.scala, which
> is
> >> not used by JUnit, but which is used by the normal broker and controller
> >> startup processes.
> >>
> >> best,
> >> Colin
> >>
> >> > Jun
> >> >
> >> > On Wed, Jun 26, 2024 at 12:52 PM Colin McCabe 
> >> wrote:
> >> >
> >> >> On Wed, Jun 26, 2024, at 12:09, Jun Rao wrote:
> >> >> > Hi, Colin,
> >> >> >
> >> >> > Thanks for restarting the discussion. A few comments.
> >> >> >
> >> >> > 1. "An unstable RPC version can be changed at any time, until it
> >> becomes
> >> >> > stable."
> >> >> >
> >> >> > What's our recommendation to non-java developers? Should they start
> >> >> > building a new version of an RPC until it is stable?
> >> >> >
> >> >>
> >> >> Hi Jun,
> >> >>
> >> >> Non-Java developers will always be using only stable APIs. Unstable
> APIs
> >> >> are only available to JUnit tests (that run inside the JUnit JVM).
> >> >>
> >> >> > Should we explicitly mark unstable versions of PRC in
> >> >> > https://kafka.apache.org/protocol.html? Currently, it's not clear
> >> which
> >> >> > versions are unstable.
> >> >> >
> >> >>
> >> >> Hmm, I don't think the unstable APIs should be documented at all in
> our
> >> >> public docs. Since they're just "possibilities for the future" that
> >> haven't
> >> >> actually been released.
> >> >>
> >> >> > 2. enable.unstable.features: Our current convention is to put
> enable
> >> in
> >> >> the
> >> >> > suffix in config names.
> >> >> >
> >> >>
> >> >> OK. I changed it to "unstable.features.enable"
> >> >>
> >> >> > 3. It would be useful to explicitly mention the removal of the
> >> following
> >> >> > two configs in the public interfaces section.
> >> >> > unstable.api.versions.enable
> >> >> > unstable.feature.versions.enable
> >> >> >
> >> >>
> >> >> OK. I added this to that section.
> >> >>
> >> >> > 4. "Clusters can be created with unstable MVs, but only in JUnit
> >> tests."
> >> >> > Hmm, we should allow developers to test unstabl

Re: [DISCUSS] KIP-1014: Managing Unstable Features in Apache Kafka

2024-06-28 Thread David Jacot
Hi Colin,

I agree that we try hard to avoid breaking compatibility. I am not
questioning this at all. I also agree with your concern.

My point was about requiring users to opt-in for a feature vs having it
enabled by default during the EA and the Preview phases. With KIP-1022, the
only way to use a non-default version of a feature is to set the unstable
config. I think that it would be nice to have the default feature version,
new stable version(s) that users could opt-in to enable during EA/Preview,
and the development one(s) gated by the unstable flag. This is perhaps more
a discussion for KIP-1022.

Best,
David

On Fri, Jun 28, 2024 at 7:50 AM Chia-Ping Tsai  wrote:

> On Wed, Jun 26, 2024, at 13:16, Jun Rao wrote:
>
> > > Hi, Colin,
> > >
> > > Thanks for the reply.
> > >
> > > 1.
> >
> https://kafka.apache.org/protocol.html#The_Messages_ConsumerGroupDescribe
> > > lists ConsumerGroupDescribeRequest, whose latest version is unstable.
> > >
> >
> > Hi Jun,
> >
> > I think that is a bug.
> >
>
> I file a jira for this (https://issues.apache.org/jira/browse/KAFKA-17051)
>


Re: [VOTE] KIP-1022 Formatting and Updating Features

2024-06-30 Thread David Jacot
ne Olshan wrote:
> > >>> >> > > > > Thanks Colin,
> > >>> >> > > > >
> > >>> >> > > > > This makes sense to me. Namely in the case where we
> perhaps
> > >>> don't
> > >>> >> > want to
> > >>> >> > > > > support version 0 anymore, we need the range to be able to
> > not
> > >>> >> > include 0.
> > >>> >> > > > > (In other words, we can't assume 0 is supported)
> > >>> >> > > > > It is unfortunate that this change is a bit tricky, but I
> > think
> > >>> >> it's
> > >>> >> > the
> > >>> >> > > > > best option.
> > >>> >> > > > >
> > >>> >> > > > > Can you clarify
> > >>> >> > > > >> The server will simply leave out the features whose
> minimum
> > >>> >> > supported
> > >>> >> > > > > value is 0 for clients that send v3
> > >>> >> > > > >
> > >>> >> > > > > For 3.8, I planned to set the 0s in the response to 1. Is
> it
> > >>> better
> > >>> >> > to
> > >>> >> > > > > suppress the zero version features in the response so we
> are
> > >>> >> > consistent
> > >>> >> > > > > between trunk and 3.8?
> > >>> >> > > > >
> > >>> >> > > > > Thanks,
> > >>> >> > > > > Justine
> > >>> >> > > > >
> > >>> >> > > > > On Fri, Jun 21, 2024 at 4:34 PM Colin McCabe <
> > >>> cmcc...@apache.org>
> > >>> >> > wrote:
> > >>> >> > > > >
> > >>> >> > > > >> Hi all,
> > >>> >> > > > >>
> > >>> >> > > > >> It seems that there was a bug in older versions of Kafka
> > which
> > >>> >> > caused
> > >>> >> > > > >> deserialization problems when a supported feature range
> > >>> included
> > >>> >> 0.
> > >>> >> > For
> > >>> >> > > > >> example, the range for group.version of [0, 1] would be a
> > >>> problem
> > >>> >> in
> > >>> >> > > > this
> > >>> >> > > > >> situation.
> > >>> >> > > > >>
> > >>> >> > > > >> This obviously makes supportedVersions kind of useless.
> Any
> > >>> >> feature
> > >>> >> > that
> > >>> >> > > > >> doesn't exist today is effectively at v0 today (v0 is
> > >>> equivalent
> > >>> >> to
> > >>> >> > > > "off").
> > >>> >> > > > >> But if we can't declare that the server supports [0, 1]
> or
> > >>> >> similar,
> > >>> >> > we
> > >>> >> > > > >> can't declare that it supports the feature being off.
> > >>> Therefore,
> > >>> >> no
> > >>> >> > > > rolling
> > >>> >> > > > >> upgrades are possible.
> > >>> >> > > > >>
> > >>> >> > > > >> We noticed this bug during the 3.8 release when we
> noticed
> > >>> >> problems
> > >>> >> > in
> > >>> >> > > > >> upgrade tests. As an addendum to KIP-1022, we're adding
> the
> > >>> >> > following
> > >>> >> > > > >> solution:
> > >>> >> > > > >>
> > >>> >> > > > >> - There will be a new v4 for ApiVersionsRequest
> > >>> >> > > > >>
> > >>> >> > > > >> - Clients that sent v4 will promise to correctly handle
> > ranges
> > >>> >> that
> > >>> >> > > > start
> > >>> >> > > > >> with 0, such as [0, 1]
> > >>> >> > > > >>
> > >>> >> > > > >> - The server will simply leave out the features whose
> > minimum
> > >>> >> > supported
> > >>> >> > > > >> value is 0 for clients that send v3
> > >>> >> > > > >>
> > >>> >> > > > >> - ApiVersionsRequest v4 will be supported in AK 3.9 and
> > >>> above. AK
> > >>> >> > 3.8
> > >>> >> > > > will
> > >>> >> > > > >> ship with ApiVersionsRequest v3 just as today.
> > >>> >> > > > >>
> > >>> >> > > > >> thanks,
> > >>> >> > > > >> Colin
> > >>> >> > > > >>
> > >>> >> > > > >>
> > >>> >> > > > >> On Mon, Apr 15, 2024, at 11:01, Justine Olshan wrote:
> > >>> >> > > > >> > Hey folks,
> > >>> >> > > > >> >
> > >>> >> > > > >> > Thanks everyone! I will go ahead and call it.
> > >>> >> > > > >> > The KIP passes with the following +1 votes:
> > >>> >> > > > >> >
> > >>> >> > > > >> > - Andrew Schofield (non-binding)
> > >>> >> > > > >> > - David Jacot (binding)
> > >>> >> > > > >> > - José Armando García Sancio (binding)
> > >>> >> > > > >> > - Jun Rao (binding)
> > >>> >> > > > >> >
> > >>> >> > > > >> > Thanks again,
> > >>> >> > > > >> > Justine
> > >>> >> > > > >> >
> > >>> >> > > > >> > On Fri, Apr 12, 2024 at 11:16 AM Jun Rao
> > >>> >>  > >>> >> > >
> > >>> >> > > > >> wrote:
> > >>> >> > > > >> >
> > >>> >> > > > >> >> Hi, Justine,
> > >>> >> > > > >> >>
> > >>> >> > > > >> >> Thanks for the KIP. +1
> > >>> >> > > > >> >>
> > >>> >> > > > >> >> Jun
> > >>> >> > > > >> >>
> > >>> >> > > > >> >> On Wed, Apr 10, 2024 at 9:13 AM José Armando García
> > Sancio
> > >>> >> > > > >> >>  wrote:
> > >>> >> > > > >> >>
> > >>> >> > > > >> >> > Hi Justine,
> > >>> >> > > > >> >> >
> > >>> >> > > > >> >> > +1 (binding)
> > >>> >> > > > >> >> >
> > >>> >> > > > >> >> > Thanks for the improvement.
> > >>> >> > > > >> >> > --
> > >>> >> > > > >> >> > -José
> > >>> >> > > > >> >> >
> > >>> >> > > > >> >>
> > >>> >> > > > >>
> > >>> >> > > >
> > >>> >> >
> > >>> >> >
> > >>> >> >
> > >>> >> > --
> > >>> >> > -José
> > >>> >> >
> > >>> >>
> > >>>
> > >>
> >
>


Re: [DISCUSS] KIP-1062: Introduce Pagination for some requests used by Admin API

2024-07-02 Thread David Jacot
Hi Omnia,

Thanks for the KIP. I agree that we should migrate admin APIs to the new
pattern.

DJ1: Why do we want to migrate only a subset of the APIs vs migrating all
of them? For instance, there are DescribeGroups, ConsumerGroupDescribe,
etc. Do we have reasons not to migrate them too? I think that it would be
great to have a KIP that establishes the pattern for all the admin APIs.

DJ2: I am not a fan of all the new parameters passed to the tools
(e.g. --partition-size-limit-per-response). I wonder if we could rather
have a config in the admin client to set the default page size for all
requests.

DJ3: I assume that the Admin client will transparently handle the change.
It would be great to call it out in the compatibility section.

Thanks,
David

On Tue, Jul 2, 2024 at 11:17 AM Andrew Schofield 
wrote:

> Hi,
> Thanks for the response. Makes sense to me. Just one additional comment:
>
> AS5: The cursor for ListGroupsResponse is called `TransactionalCursor`
> which
> seems like a copy-paste mistake.
>
> Thanks,
> Andrew
>
> > On 30 Jun 2024, at 22:28, Omnia Ibrahim  wrote:
> >
> > Hi Andrew thanks for having a look into the KIP
> >
> >> AS1: Besides topics, the most numerous resources in Kafka clusters in
> my experience
> >> are consumer groups. Would it be possible to extend the KIP to cover
> ListGroups while
> >> you’re in here? I’ve heard of clusters with truly vast numbers of
> groups. This is also
> >> potentially a sign of a misbehaving or poorly written clients. Getting
> a page of groups
> >> with a massive ItemsLeftToFetch would be nice.
> > Yes, I also had few experiences with large cluster where to list
> consumer groups can take up to 5min. I update the KIP to include this as
> well.
> >
> >> AS2: A tiny nit: The versions for the added fields are incorrect in
> some cases.
> > I believe I fixed all of them now
> >
> >> AS3: I don’t quite understand the cursor for
> OffsetFetchRequest/Response.
> >> It looks like the cursor is (topic, partition), but not group ID. Does
> the cursor
> >> apply to all groups in the request, or is group ID missing?
> >
> > I was thinking that the last one in the response will be the one that
> has the cursor while the rest will have null. But if we are moving
> NextCursour to the top level of the response then the cursor will need
> groupID.
> >> AS4: For the remaining request/response pairs, the cursor makes sense
> to me,
> >> but I do wonder whether `NextCursor` should be at the top level of the
> responses
> >> instead, like DescribeTopicPartitionsResponse.
> >
> > Updates the KIP to reflect this now.
> >
> > Let me know if you have any more feedback on this.
> >
> > Best
> > Omnia
> >
> >> On 27 Jun 2024, at 17:53, Andrew Schofield 
> wrote:
> >>
> >> Hi Omnia,
> >> Thanks for the KIP. This is a really nice improvement for administering
> large clusters.
> >>
> >> AS1: Besides topics, the most numerous resources in Kafka clusters in
> my experience
> >> are consumer groups. Would it be possible to extend the KIP to cover
> ListGroups while
> >> you’re in here? I’ve heard of clusters with truly vast numbers of
> groups. This is also
> >> potentially a sign of a misbehaving or poorly written clients. Getting
> a page of groups
> >> with a massive ItemsLeftToFetch would be nice.
> >>
> >> AS2: A tiny nit: The versions for the added fields are incorrect in
> some cases.
> >>
> >> AS3: I don’t quite understand the cursor for
> OffsetFetchRequest/Response.
> >> It looks like the cursor is (topic, partition), but not group ID. Does
> the cursor
> >> apply to all groups in the request, or is group ID missing?
> >>
> >> AS4: For the remaining request/response pairs, the cursor makes sense
> to me,
> >> but I do wonder whether `NextCursor` should be at the top level of the
> responses
> >> instead, like DescribeTopicPartitionsResponse.
> >>
> >> Thanks,
> >> Andrew
> >>
> >>> On 27 Jun 2024, at 14:05, Omnia Ibrahim 
> wrote:
> >>>
> >>> Hi everyone, I would like to start a discussion thread for KIP-1062
> >>>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1062%3A+Introduce+Pagination+for+some+requests+used+by+Admin+API
> >>>
> >>>
> >>> Thanks
> >>> Omnia
>
>
>


Re: [VOTE] KIP-1022 Formatting and Updating Features

2024-07-03 Thread David Jacot
Hi Jun, Colin,

Thanks for your replies.

If the FeatureCommand relies on version 0 too, my suggestion does not work.
Omitting the features for old clients as suggested by Colin seems fine for
me. In practice, administrators will usually use a version of
FeatureCommand matching the cluster version so the impact is not too bad
knowing that the first features will be introduced from 3.9 on.

Best,
David

On Tue, Jul 2, 2024 at 2:15 AM Colin McCabe  wrote:

> Hi David,
>
> In the ApiVersionsResponse, we really don't have an easy way of mapping
> finalizedVersion = 1 to "off" in older releases such as 3.7.0. For example,
> if a 3.9.0 broker advertises that it has finalized group.version = 1, that
> will be treated by 3.7.0 as a brand new feature, not as "KIP-848 is off."
> However, I suppose we could work around this by not setting a
> finalizedVersion at all for group.version (or any other feature) if its
> finalized level was 1. We could also work around the "deletion = set to 0"
> issue on the server side. The server can translate requests to set the
> finalized level to 0, into requests to set it to 1.
>
> So maybe this solution is worth considering, although it's unfortunate to
> lose 0. I suppose we'd have to special case metadata.version being set to
> 1, since that was NOT equivalent to it being "off"
>
> best,
> Colin
>
>
> On Mon, Jul 1, 2024, at 10:11, Jun Rao wrote:
> > Hi, David,
> >
> > Yes, that's another option. It probably has its own challenges. For
> > example, the FeatureCommand tool currently treats disabling a feature as
> > setting the version to 0. It would be useful to get Jose's opinion on
> this
> > since he introduced version 0 in the kraft.version feature.
> >
> > Thanks,
> >
> > Jun
> >
> > On Sun, Jun 30, 2024 at 11:48 PM David Jacot  >
> > wrote:
> >
> >> Hi Jun, Colin,
> >>
> >> Have we considered sticking with the range going from version 1 to N
> where
> >> version 1 would be the equivalent of "disabled"? In the group.version
> case,
> >> we could introduce group.version=1 that does basically nothing and
> >> group.version=2 that enables the new protocol. I suppose that we could
> do
> >> the same for the other features. I agree that it is less elegant but it
> >> would avoid all the backward compatibility issues.
> >>
> >> Best,
> >> David
> >>
> >> On Fri, Jun 28, 2024 at 6:02 PM Jun Rao 
> wrote:
> >>
> >> > Hi, Colin,
> >> >
> >> > Yes, #3 is the scenario that I was thinking about.
> >> >
> >> > In either approach, there will be some information missing in the old
> >> > client. It seems that we should just pick the one that's less wrong.
> In
> >> the
> >> > more common case when a feature is finalized on the server,
> presenting a
> >> > supported feature with a range of 1-1 seems less wrong than omitting
> it
> >> in
> >> > the output of "kafka-features describe".
> >> >
> >> > Thanks,
> >> >
> >> > Jun
> >> >
> >> > On Thu, Jun 27, 2024 at 9:52 PM Colin McCabe 
> wrote:
> >> >
> >> > > Hi Jun,
> >> > >
> >> > > This is a fair question. I think there's a few different scenarios
> to
> >> > > consider:
> >> > >
> >> > > 1. mixed server software versions in a single cluster
> >> > >
> >> > > 2. new client software + old server software
> >> > >
> >> > > 3. old client software + new server software
> >> > >
> >> > > In scenario #1 and #2, we have old (pre-3.9) server software in the
> >> mix.
> >> > > This old software won't support features like group.version and
> >> > > kraft.version. As we know, there are no features supported in 3.8
> and
> >> > older
> >> > > except metadata.version itself. So the fact that we leave out some
> >> stuff
> >> > > from the ApiVersionResponse isn't terribly significant. We weren't
> >> going
> >> > to
> >> > > be able to enable those post-3.8 features anyway, since enabling a
> >> > feature
> >> > > requires ALL server nodes to support it.
> >> > >
> >> > > Scenario #3 is more interesting. With new server software, features
> >> like
> >> > 

Re: [DISCUSS] Graduation steps for Features

2024-07-31 Thread David Jacot
Hi Josep,

Thanks for starting the discussion.

We used Early Access, Preview and GA (or Production Ready) for KIP-848 and
I find it pretty nice. We could add the tentative release plan to the KIP's
header and it could be used as the source of truth.

Best,
David

On Wed, Jul 31, 2024 at 11:53 AM Josep Prat 
wrote:

> Hi Andrew,
> I can definitely write a KIP, but before doing so I'd like to gather some
> feedback from the community around these steps and how they are perceived
> by different groups of people.
>
> On Wed, Jul 31, 2024 at 11:50 AM Andrew Schofield <
> andrew_schofi...@live.com>
> wrote:
>
> > Hi Josep,
> > I think it’s high time that this was tackled. I suggest that it would be
> > best
> > handled as a KIP because then we have a document which can be discussed
> > and improved, followed by a formal vote.
> >
> > A standard set of terms with agreed meanings would be very helpful for
> > some of the larger KIPs which take many releases to be properly ready for
> > prime time. Most KIPs don’t need this, but a handful definitely do.
> >
> > Personally, I like the sequence that KIP-848 has taken, moving from Early
> > Access, to Preview, and finally complete. I intend to follow the same
> > sequence
> > for KIP-932.
> >
> > Thanks,
> > Andrew
> >
> > > On 31 Jul 2024, at 10:15, Josep Prat 
> > wrote:
> > >
> > > Also as part of this discussion I would like to flag that we need to be
> > > able to know how we can flag this properly so it's known for the
> Release
> > > Manager.
> > > For example, a KIP is approved, the Jira associated with it is being
> > worked
> > > on. Release happens, Jira is still open, how can we flag that this KIP
> is
> > > in early access, or preview?
> > >
> > > Best,
> > >
> > > On Wed, Jul 31, 2024 at 11:03 AM Josep Prat 
> wrote:
> > >
> > >> Hi Kafka devs,
> > >>
> > >> Lately we started using "early access", "production ready" and also
> > >> "preview" to determine the grade of "production readiness" of the
> > features
> > >> we deliver to our community.
> > >> However, as far as I know, there is no official definition from the
> > Apache
> > >> Kafka side on which are the graduation steps for features and what
> type
> > of
> > >> "guarantees" each of these offer.
> > >>
> > >> I think we should agree on which terms we should use and what each of
> > >> these exactly mean in terms of reliability. So far it seems we have
> this
> > >> graduation steps:
> > >> - Early Access: Feature is just complete but not yet fully polished
> and
> > >> maybe not used in production in many environments
> > >> - Preview: Feature was early access before and it underwent at least a
> > >> cycle of improvements and fixes and it's used in some production
> > >> environments maybe
> > >> - Production ready: Feature is officially released and it fulfills the
> > >> expected initial needs
> > >>
> > >> Note that we don't offer any guarantees or SLA/SLO in the classical
> > term.
> > >>
> > >> Is this something we can agree on? What do those terms mean to you? Do
> > we
> > >> need more steps? Or do we need less steps?
> > >>
> > >> Best,
> > >> --
> > >> [image: Aiven] 
> > >>
> > >> *Josep Prat*
> > >> Open Source Engineering Director, *Aiven*
> > >> josep.p...@aiven.io   |   +491715557497
> > >> aiven.io    |
> > >> 
> > >>    <
> > https://twitter.com/aiven_io>
> > >> *Aiven Deutschland GmbH*
> > >> Alexanderufer 3-7, 10117 Berlin
> > >> Geschäftsführer: Oskari Saarenmaa, Hannu Valtonen,
> > >> Anna Richardson, Kenneth Chen
> > >> Amtsgericht Charlottenburg, HRB 209739 B
> > >>
> > >
> > >
> > > --
> > > [image: Aiven] 
> > >
> > > *Josep Prat*
> > > Open Source Engineering Director, *Aiven*
> > > josep.p...@aiven.io   |   +491715557497
> > > aiven.io    |   <
> > https://www.facebook.com/aivencloud>
> > >     <
> > https://twitter.com/aiven_io>
> > > *Aiven Deutschland GmbH*
> > > Alexanderufer 3-7, 10117 Berlin
> > > Geschäftsführer: Oskari Saarenmaa, Hannu Valtonen,
> > > Anna Richardson, Kenneth Chen
> > > Amtsgericht Charlottenburg, HRB 209739 B
> >
> >
>
> --
> [image: Aiven] 
>
> *Josep Prat*
> Open Source Engineering Director, *Aiven*
> josep.p...@aiven.io   |   +491715557497
> aiven.io    |    >
>      <
> https://twitter.com/aiven_io>
> *Aiven Deutschland GmbH*
> Alexanderufer 3-7, 10117 Berlin
> Geschäftsführer: Oskari Saarenmaa, Hannu Valtonen,
> Anna Richardson, Kenneth Chen
> Amtsgericht Charlottenburg, HRB 209739 B
>


Re: [DISCUSS] Graduation steps for Features

2024-07-31 Thread David Jacot
We could also use a hierarchie: KIP Parent Jira > Milestones Jiras > Tasks.

Best,
David

On Wed, Jul 31, 2024 at 4:22 PM Josep Prat 
wrote:

> Hi David,
>
> One of the problems I see is that the KIP index page has a 1-to-many
> relationship between KIP and release. I guess we might want to turn this to
> a many-to-many qualified relationship. Otherwise it might be complicated
> for the release manager or the KIP driver(s) to keep the Release Plan page
> up-to-date for the different steps.
>
> Another alternative would be to have special sub-tasks in JIRA that would
> indicate the state of the KIP, then using the "fixed version" label they'll
> be included in the release notes and the Release Manager can look for these
> special ones when writing announcements or making sure the release notes
> are up-to-date.
>
> Best,
>
> On Wed, Jul 31, 2024 at 3:54 PM David Jacot 
> wrote:
>
> > Hi Josep,
> >
> > Thanks for starting the discussion.
> >
> > We used Early Access, Preview and GA (or Production Ready) for KIP-848
> and
> > I find it pretty nice. We could add the tentative release plan to the
> KIP's
> > header and it could be used as the source of truth.
> >
> > Best,
> > David
> >
> > On Wed, Jul 31, 2024 at 11:53 AM Josep Prat  >
> > wrote:
> >
> > > Hi Andrew,
> > > I can definitely write a KIP, but before doing so I'd like to gather
> some
> > > feedback from the community around these steps and how they are
> perceived
> > > by different groups of people.
> > >
> > > On Wed, Jul 31, 2024 at 11:50 AM Andrew Schofield <
> > > andrew_schofi...@live.com>
> > > wrote:
> > >
> > > > Hi Josep,
> > > > I think it’s high time that this was tackled. I suggest that it would
> > be
> > > > best
> > > > handled as a KIP because then we have a document which can be
> discussed
> > > > and improved, followed by a formal vote.
> > > >
> > > > A standard set of terms with agreed meanings would be very helpful
> for
> > > > some of the larger KIPs which take many releases to be properly ready
> > for
> > > > prime time. Most KIPs don’t need this, but a handful definitely do.
> > > >
> > > > Personally, I like the sequence that KIP-848 has taken, moving from
> > Early
> > > > Access, to Preview, and finally complete. I intend to follow the same
> > > > sequence
> > > > for KIP-932.
> > > >
> > > > Thanks,
> > > > Andrew
> > > >
> > > > > On 31 Jul 2024, at 10:15, Josep Prat 
> > > > wrote:
> > > > >
> > > > > Also as part of this discussion I would like to flag that we need
> to
> > be
> > > > > able to know how we can flag this properly so it's known for the
> > > Release
> > > > > Manager.
> > > > > For example, a KIP is approved, the Jira associated with it is
> being
> > > > worked
> > > > > on. Release happens, Jira is still open, how can we flag that this
> > KIP
> > > is
> > > > > in early access, or preview?
> > > > >
> > > > > Best,
> > > > >
> > > > > On Wed, Jul 31, 2024 at 11:03 AM Josep Prat 
> > > wrote:
> > > > >
> > > > >> Hi Kafka devs,
> > > > >>
> > > > >> Lately we started using "early access", "production ready" and
> also
> > > > >> "preview" to determine the grade of "production readiness" of the
> > > > features
> > > > >> we deliver to our community.
> > > > >> However, as far as I know, there is no official definition from
> the
> > > > Apache
> > > > >> Kafka side on which are the graduation steps for features and what
> > > type
> > > > of
> > > > >> "guarantees" each of these offer.
> > > > >>
> > > > >> I think we should agree on which terms we should use and what each
> > of
> > > > >> these exactly mean in terms of reliability. So far it seems we
> have
> > > this
> > > > >> graduation steps:
> > > > >> - Early Access: Feature is just complete but not yet fully
> polished
> > > and
> > > > >> maybe not used in production in many environm

  1   2   3   4   5   6   7   8   9   10   >