Re: KIP-560 Discuss

[Boyang Chen]

Thanks Sophie for the explanation! I read Sang's PR and basically he did
exactly what you proposed (check it here
 in case I'm wrong).

I think Sophie's response answers Gwen's question already, while in the
meantime for a KIP itself we are not required to mention all the internal
details about how to make the changes happen (like how to actually get the
external topics), considering the change scope is pretty small as well. But
again, it would do no harm if we mention it inside Proposed Change session
specifically so that people won't get confused about how.


On Thu, Jan 23, 2020 at 8:26 PM Sophie Blee-Goldman 
wrote:

> Hi all,
>
> I think what Gwen is trying to ask (correct me if I'm wrong) is how we can
> infer which topics are associated with
> Streams from the admin client's topic list. I agree that this doesn't seem
> possible, since as she pointed out the
> topics list (or even description) lacks the specific information we need.
>
> What we could do instead is use the admin client's
> `describeConsumerGroups` API to get the information
> on the Streams app's consumer group specifically -- note that the Streams
> application.id config is also used
> as the consumer group id, so each app forms a group to read from the input
> topics. We could compile a list
> of these topics just by looking at each member's assignment (and even check
> for a StreamsPartitionAssignor
> to verify that this is indeed a Streams app group, if we're being
> paranoid).
>
> The reset tool actually already gets the consumer group description, in
> order to validate there are no active
> consumers in the group. We may as well grab the list of topics from it
> while it's there. Or did you have something
> else in mind?
>
> On Sat, Jan 18, 2020 at 6:17 PM Sang wn Lee  wrote:
>
> > Thank you
> >
> > I understand you
> >
> > 1. admin client has topic list
> > 2. applicationId can only have one stream, so It won't be a problem!
> > 3. For example, --input-topic [reg]
> > Allowing reg solves some inconvenience
> >
> >
> > On 2020/01/18 18:15:23, Gwen Shapira  wrote:
> > > I am not sure I follow. Afaik:
> > >
> > > 1. Topics don't include client ID information
> > > 2. Even if you did, the same ID could be used for topics that are not
> > Kafka
> > > Streams input
> > >
> > > The regex idea sounds doable, but I'm not sure it solves much?
> > >
> > >
> > > On Sat, Jan 18, 2020, 7:12 AM Sang wn Lee 
> wrote:
> > >
> > > > Thank you
> > > > Gwen Shapira!
> > > > We'll add a flag to clear all topics by clientId
> > > > It is ‘reset-all-external-topics’
> > > >
> > > > I also want to use regex on the input topic flag to clear all
> matching
> > > > topics.
> > > >
> > > > On 2020/01/17 19:29:09, Gwen Shapira  wrote:
> > > > > Seem like a very nice improvement to me. But I have to admit that I
> > > > > don't understand how this will how - how could you infer the input
> > > > > topics?
> > > > >
> > > > > On Thu, Jan 16, 2020 at 10:03 AM Sang wn Lee  >
> > > > wrote:
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > Starting this thread to discuss KIP-560:
> > > > > > wiki link :
> > > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-560%3A+Auto+infer+external+topic+partitions+in+stream+reset+tool
> > > > > >
> > > > > > I'm newbie
> > > > > > I would like to receive feedback on the following features!
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [VOTE] KIP-559: Make the Kafka Protocol Friendlier with L7 Proxies

[Satish Duggana]

+1 (non-binding)

On Fri, Jan 24, 2020 at 11:10 AM Harsha Chintalapani  wrote:
>
> +1 ( binding). Much needed!
> -Harsha
>
>
> On Thu, Jan 23, 2020 at 7:17 PM, Guozhang Wang  wrote:
>
> > +1 （binding）
> >
> > On Thu, Jan 23, 2020 at 1:55 PM Guozhang Wang  wrote:
> >
> > Yeah that makes sense, it is a good-to-have if we can push through this in
> > 2.5 but if we do not have bandwidth that's fine too :)
> >
> > Guozhang
> >
> > On Thu, Jan 23, 2020 at 1:40 PM David Jacot  wrote:
> >
> > Hi Guozhang,
> >
> > Thank you for your input.
> >
> > 1) You're right. I've put it there due to the version bump only. I'll make
> > it clearer.
> >
> > 2) I'd rather prefer to keep the scope as it is because 1) that field is
> > not related to
> > the problem that we are solving here and 2) I am not sure that I will have
> > the
> > bandwidth to do this before the feature freeze. The PR is already ready.
> > That being
> > said, as the addition of that field is part of KIP-429 and KIP-429 has
> > already been
> > accepted, we could give it a shot to avoid having to bump the version
> > twice. I could
> > try putting together a PR before the feature freeze but without guarantee.
> > Does that
> > make sense?
> >
> > David
> >
> > On Thu, Jan 23, 2020 at 9:44 AM Guozhang Wang  wrote:
> >
> > Hello David,
> >
> > Thanks for the KIP! I have read through the proposal and had one minor
> >
> > and
> >
> > one meta comment. But overall it looks good to me!
> >
> > 1) The JoinGroupRequest format does not have any new fields proposed,
> >
> > so we
> >
> > could either clarify that it is listed here but without modifications
> >
> > (only
> >
> > version bumps) or just remove it from the wiki.
> >
> > 2) Could we consider adding a "protocol version" to allow brokers to
> >
> > select
> >
> > the leader with the highest version? This thought is brought up in
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/
> > KIP-429%3A+Kafka+Consumer+Incremental+Rebalance+Protocol#KIP-429:KafkaConsumerIncrementalRebalanceProtocol-LookingintotheFuture:AssignorVersion
> >
> > .
> > I'm fine with keeping this KIP's scope as is, just wondering if you feel
> > comfortable piggy-backing this change as well if we are going to bump up
> > the JoinGroupReq/Response anyways.
> >
> > Guozhang
> >
> > On Wed, Jan 22, 2020 at 9:10 AM Eno Thereska 
> > wrote:
> >
> > This is awesome! +1 (non binding)
> > Eno
> >
> > On Tue, Jan 21, 2020 at 10:00 PM Gwen Shapira 
> >
> > wrote:
> >
> > Thank you for the KIP. Awesomely cloud-native improvement :)
> >
> > +1 (binding)
> >
> > On Tue, Jan 21, 2020, 9:35 AM David Jacot 
> >
> > wrote:
> >
> > Hi all,
> >
> > I would like to start a vote on KIP-559: Make the Kafka Protocol
> >
> > Friendlier
> >
> > with L7 Proxies.
> >
> > The KIP is here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/
> > KIP-559%3A+Make+the+Kafka+Protocol+Friendlier+with+L7+Proxies
> >
> > Thanks,
> > David
> >
> > --
> > -- Guozhang
> >
> > --
> > -- Guozhang
> >
> > --
> > -- Guozhang
> >

Re: [VOTE] KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[Navinder Brar]

Thanks, everyone for voting and discussions. The KIP is passed with 3 binding 
votes from John, Matthias, and Guozhang and 1 non-binding vote from Bruno. 

On Friday, 24 January, 2020, 08:50:21 am IST, Bruno Cadonna 
 wrote:  
 
 Hi Navinder,

+1 (non-binding)

Best,
Bruno

On Thu, Jan 23, 2020 at 9:19 AM John Roesler  wrote:
>
> Thanks, Navinder. It's just to give everyone a chance to object if they 
> wanted to.
> -John
>
> On Thu, Jan 23, 2020, at 00:44, Navinder Brar wrote:
> > Oh sorry, my bad. Will wait for another 12 hours.
> >
> >    On Thursday, 23 January, 2020, 12:09:57 pm IST, Matthias J. Sax
> >  wrote:
> >
> >  Navinder,
> >
> > a KIP vote must be open for 72h and cannot be closed earlier.
> >
> > -Matthias
> >
> > On 1/22/20 10:27 PM, Navinder Brar wrote:
> > > Thanks, everyone for voting. KIP-562 has been accepted with binding votes 
> > > from John, Matthias, and Guozhang.
> > >
> > >    On Thursday, 23 January, 2020, 09:40:07 am IST, Guozhang Wang 
> > > wrote:
> > >
> > >  +1 (binding) from me as well.
> > >
> > > On Wed, Jan 22, 2020 at 5:59 PM Matthias J. Sax 
> > > wrote:
> > >
> > >> I have a few minor comments (compare the DISCUSS thread), but overall
> > >> the KIP looks good.
> > >>
> > >> +1 (binding)
> > >>
> > >>
> > >> -Matthias
> > >>
> > >> On 1/22/20 10:09 AM, John Roesler wrote:
> > >>> Thanks for updating the KIP, Navinder.
> > >>>
> > >>> I'm +1 (binding) on the current proposal
> > >>>
> > >>> Thanks,
> > >>> -John
> > >>>
> > >>> On Tue, Jan 21, 2020, at 12:50, Navinder Brar wrote:
> >  Thanks, Guozhang. I agree it makes total sense. I will make the
> >  edits.~Navinder
> > 
> >     On Tuesday, 21 January, 2020, 11:00:32 pm IST, Guozhang Wang
> >   wrote:
> > 
> >   Hello Navinder,
> > 
> >  Thanks for brining up this proposal. I made a quick pass on that and
> >  overall I think I agree with your ideas. Just a few thoughts about the
> >  public APIs:
> > 
> >  1) As we are adding a new overload to `KafkaStreams#store`, could we
> > >> just
> >  add the storeName and queryableStoreType as part of StoreQueryParam, 
> >  and
> >  leaving that the only parameter of the function?
> > 
> >  2) along with 1), for the static constructors, instead of iterating 
> >  over
> >  all possible combos I'd suggest we make constructors with only, say,
> >  storeName, and then adding `withXXX()` setters to set other fields.
> > >> This is
> >  in case we want to add more param fields into the object, that we do 
> >  not
> >  need to exponentially adding and deprecating the static constructors.
> > 
> > 
> >  Guozhang
> > 
> > 
> >  On Mon, Jan 20, 2020 at 10:42 AM Navinder Brar
> >   wrote:
> > 
> > > Hello all,
> > >
> > > I'd like to propose a vote to serve keys from a specific
> > >> partition-store
> > > instead of iterating over all the local stores of an instance to
> > >> locate the
> > > key, as which happens currently.
> > > The full KIP is provided here:
> > >
> > >
> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-562%3A+Allow+fetching+a+key+from+a+single+partition+rather+than+iterating+over+all+the+stores+on+an+instance
> > >
> > >
> > > Thanks,
> > > Navinder
> > >
> > 
> > 
> >  --
> >  -- Guozhang
> > 
> > >>
> > >>
> > >
> >
  

Re: [VOTE] KIP-559: Make the Kafka Protocol Friendlier with L7 Proxies

[Harsha Chintalapani]

+1 ( binding). Much needed!
-Harsha


On Thu, Jan 23, 2020 at 7:17 PM, Guozhang Wang  wrote:

> +1 （binding）
>
> On Thu, Jan 23, 2020 at 1:55 PM Guozhang Wang  wrote:
>
> Yeah that makes sense, it is a good-to-have if we can push through this in
> 2.5 but if we do not have bandwidth that's fine too :)
>
> Guozhang
>
> On Thu, Jan 23, 2020 at 1:40 PM David Jacot  wrote:
>
> Hi Guozhang,
>
> Thank you for your input.
>
> 1) You're right. I've put it there due to the version bump only. I'll make
> it clearer.
>
> 2) I'd rather prefer to keep the scope as it is because 1) that field is
> not related to
> the problem that we are solving here and 2) I am not sure that I will have
> the
> bandwidth to do this before the feature freeze. The PR is already ready.
> That being
> said, as the addition of that field is part of KIP-429 and KIP-429 has
> already been
> accepted, we could give it a shot to avoid having to bump the version
> twice. I could
> try putting together a PR before the feature freeze but without guarantee.
> Does that
> make sense?
>
> David
>
> On Thu, Jan 23, 2020 at 9:44 AM Guozhang Wang  wrote:
>
> Hello David,
>
> Thanks for the KIP! I have read through the proposal and had one minor
>
> and
>
> one meta comment. But overall it looks good to me!
>
> 1) The JoinGroupRequest format does not have any new fields proposed,
>
> so we
>
> could either clarify that it is listed here but without modifications
>
> (only
>
> version bumps) or just remove it from the wiki.
>
> 2) Could we consider adding a "protocol version" to allow brokers to
>
> select
>
> the leader with the highest version? This thought is brought up in
>
> https://cwiki.apache.org/confluence/display/KAFKA/
> KIP-429%3A+Kafka+Consumer+Incremental+Rebalance+Protocol#KIP-429:KafkaConsumerIncrementalRebalanceProtocol-LookingintotheFuture:AssignorVersion
>
> .
> I'm fine with keeping this KIP's scope as is, just wondering if you feel
> comfortable piggy-backing this change as well if we are going to bump up
> the JoinGroupReq/Response anyways.
>
> Guozhang
>
> On Wed, Jan 22, 2020 at 9:10 AM Eno Thereska 
> wrote:
>
> This is awesome! +1 (non binding)
> Eno
>
> On Tue, Jan 21, 2020 at 10:00 PM Gwen Shapira 
>
> wrote:
>
> Thank you for the KIP. Awesomely cloud-native improvement :)
>
> +1 (binding)
>
> On Tue, Jan 21, 2020, 9:35 AM David Jacot 
>
> wrote:
>
> Hi all,
>
> I would like to start a vote on KIP-559: Make the Kafka Protocol
>
> Friendlier
>
> with L7 Proxies.
>
> The KIP is here:
>
> https://cwiki.apache.org/confluence/display/KAFKA/
> KIP-559%3A+Make+the+Kafka+Protocol+Friendlier+with+L7+Proxies
>
> Thanks,
> David
>
> --
> -- Guozhang
>
> --
> -- Guozhang
>
> --
> -- Guozhang
>

Re: KIP-560 Discuss

[Sophie Blee-Goldman]

Hi all,

I think what Gwen is trying to ask (correct me if I'm wrong) is how we can
infer which topics are associated with
Streams from the admin client's topic list. I agree that this doesn't seem
possible, since as she pointed out the
topics list (or even description) lacks the specific information we need.

What we could do instead is use the admin client's
`describeConsumerGroups` API to get the information
on the Streams app's consumer group specifically -- note that the Streams
application.id config is also used
as the consumer group id, so each app forms a group to read from the input
topics. We could compile a list
of these topics just by looking at each member's assignment (and even check
for a StreamsPartitionAssignor
to verify that this is indeed a Streams app group, if we're being paranoid).

The reset tool actually already gets the consumer group description, in
order to validate there are no active
consumers in the group. We may as well grab the list of topics from it
while it's there. Or did you have something
else in mind?

On Sat, Jan 18, 2020 at 6:17 PM Sang wn Lee  wrote:

> Thank you
>
> I understand you
>
> 1. admin client has topic list
> 2. applicationId can only have one stream, so It won't be a problem!
> 3. For example, --input-topic [reg]
> Allowing reg solves some inconvenience
>
>
> On 2020/01/18 18:15:23, Gwen Shapira  wrote:
> > I am not sure I follow. Afaik:
> >
> > 1. Topics don't include client ID information
> > 2. Even if you did, the same ID could be used for topics that are not
> Kafka
> > Streams input
> >
> > The regex idea sounds doable, but I'm not sure it solves much?
> >
> >
> > On Sat, Jan 18, 2020, 7:12 AM Sang wn Lee  wrote:
> >
> > > Thank you
> > > Gwen Shapira!
> > > We'll add a flag to clear all topics by clientId
> > > It is ‘reset-all-external-topics’
> > >
> > > I also want to use regex on the input topic flag to clear all matching
> > > topics.
> > >
> > > On 2020/01/17 19:29:09, Gwen Shapira  wrote:
> > > > Seem like a very nice improvement to me. But I have to admit that I
> > > > don't understand how this will how - how could you infer the input
> > > > topics?
> > > >
> > > > On Thu, Jan 16, 2020 at 10:03 AM Sang wn Lee 
> > > wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > Starting this thread to discuss KIP-560:
> > > > > wiki link :
> > > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-560%3A+Auto+infer+external+topic+partitions+in+stream+reset+tool
> > > > >
> > > > > I'm newbie
> > > > > I would like to receive feedback on the following features!
> > > > >
> > > > >
> > > >
> > >
> >
>

Re: [VOTE] KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[Bruno Cadonna]

Hi Navinder,

+1 (non-binding)

Best,
Bruno

On Thu, Jan 23, 2020 at 9:19 AM John Roesler  wrote:
>
> Thanks, Navinder. It's just to give everyone a chance to object if they 
> wanted to.
> -John
>
> On Thu, Jan 23, 2020, at 00:44, Navinder Brar wrote:
> > Oh sorry, my bad. Will wait for another 12 hours.
> >
> > On Thursday, 23 January, 2020, 12:09:57 pm IST, Matthias J. Sax
> >  wrote:
> >
> >  Navinder,
> >
> > a KIP vote must be open for 72h and cannot be closed earlier.
> >
> > -Matthias
> >
> > On 1/22/20 10:27 PM, Navinder Brar wrote:
> > > Thanks, everyone for voting. KIP-562 has been accepted with binding votes 
> > > from John, Matthias, and Guozhang.
> > >
> > >On Thursday, 23 January, 2020, 09:40:07 am IST, Guozhang Wang 
> > >  wrote:
> > >
> > >  +1 (binding) from me as well.
> > >
> > > On Wed, Jan 22, 2020 at 5:59 PM Matthias J. Sax 
> > > wrote:
> > >
> > >> I have a few minor comments (compare the DISCUSS thread), but overall
> > >> the KIP looks good.
> > >>
> > >> +1 (binding)
> > >>
> > >>
> > >> -Matthias
> > >>
> > >> On 1/22/20 10:09 AM, John Roesler wrote:
> > >>> Thanks for updating the KIP, Navinder.
> > >>>
> > >>> I'm +1 (binding) on the current proposal
> > >>>
> > >>> Thanks,
> > >>> -John
> > >>>
> > >>> On Tue, Jan 21, 2020, at 12:50, Navinder Brar wrote:
> >  Thanks, Guozhang. I agree it makes total sense. I will make the
> >  edits.~Navinder
> > 
> >  On Tuesday, 21 January, 2020, 11:00:32 pm IST, Guozhang Wang
> >   wrote:
> > 
> >    Hello Navinder,
> > 
> >  Thanks for brining up this proposal. I made a quick pass on that and
> >  overall I think I agree with your ideas. Just a few thoughts about the
> >  public APIs:
> > 
> >  1) As we are adding a new overload to `KafkaStreams#store`, could we
> > >> just
> >  add the storeName and queryableStoreType as part of StoreQueryParam, 
> >  and
> >  leaving that the only parameter of the function?
> > 
> >  2) along with 1), for the static constructors, instead of iterating 
> >  over
> >  all possible combos I'd suggest we make constructors with only, say,
> >  storeName, and then adding `withXXX()` setters to set other fields.
> > >> This is
> >  in case we want to add more param fields into the object, that we do 
> >  not
> >  need to exponentially adding and deprecating the static constructors.
> > 
> > 
> >  Guozhang
> > 
> > 
> >  On Mon, Jan 20, 2020 at 10:42 AM Navinder Brar
> >   wrote:
> > 
> > > Hello all,
> > >
> > > I'd like to propose a vote to serve keys from a specific
> > >> partition-store
> > > instead of iterating over all the local stores of an instance to
> > >> locate the
> > > key, as which happens currently.
> > > The full KIP is provided here:
> > >
> > >
> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-562%3A+Allow+fetching+a+key+from+a+single+partition+rather+than+iterating+over+all+the+stores+on+an+instance
> > >
> > >
> > > Thanks,
> > > Navinder
> > >
> > 
> > 
> >  --
> >  -- Guozhang
> > 
> > >>
> > >>
> > >
> >

Re: [VOTE] KIP-559: Make the Kafka Protocol Friendlier with L7 Proxies

[Guozhang Wang]

+1 （binding）

On Thu, Jan 23, 2020 at 1:55 PM Guozhang Wang  wrote:

> Yeah that makes sense, it is a good-to-have if we can push through this in
> 2.5 but if we do not have bandwidth that's fine too :)
>
> Guozhang
>
> On Thu, Jan 23, 2020 at 1:40 PM David Jacot  wrote:
>
>> Hi Guozhang,
>>
>> Thank you for your input.
>>
>> 1) You're right. I've put it there due to the version bump only. I'll make
>> it clearer.
>>
>> 2) I'd rather prefer to keep the scope as it is because 1) that field is
>> not related to
>> the problem that we are solving here and 2) I am not sure that I will have
>> the
>> bandwidth to do this before the feature freeze. The PR is already ready.
>> That being
>> said, as the addition of that field is part of KIP-429 and KIP-429 has
>> already been
>> accepted, we could give it a shot to avoid having to bump the version
>> twice. I could
>> try putting together a PR before the feature freeze but without guarantee.
>> Does that
>> make sense?
>>
>> David
>>
>> On Thu, Jan 23, 2020 at 9:44 AM Guozhang Wang  wrote:
>>
>> > Hello David,
>> >
>> > Thanks for the KIP! I have read through the proposal and had one minor
>> and
>> > one meta comment. But overall it looks good to me!
>> >
>> > 1) The JoinGroupRequest format does not have any new fields proposed,
>> so we
>> > could either clarify that it is listed here but without modifications
>> (only
>> > version bumps) or just remove it from the wiki.
>> >
>> > 2) Could we consider adding a "protocol version" to allow brokers to
>> select
>> > the leader with the highest version? This thought is brought up in
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-429%3A+Kafka+Consumer+Incremental+Rebalance+Protocol#KIP-429:KafkaConsumerIncrementalRebalanceProtocol-LookingintotheFuture:AssignorVersion
>> > .
>> > I'm fine with keeping this KIP's scope as is, just wondering if you feel
>> > comfortable piggy-backing this change as well if we are going to bump up
>> > the JoinGroupReq/Response anyways.
>> >
>> >
>> > Guozhang
>> >
>> >
>> > On Wed, Jan 22, 2020 at 9:10 AM Eno Thereska 
>> > wrote:
>> >
>> > > This is awesome! +1 (non binding)
>> > > Eno
>> > >
>> > > On Tue, Jan 21, 2020 at 10:00 PM Gwen Shapira 
>> wrote:
>> > > >
>> > > > Thank you for the KIP. Awesomely cloud-native improvement :)
>> > > >
>> > > > +1 (binding)
>> > > >
>> > > >
>> > > > On Tue, Jan 21, 2020, 9:35 AM David Jacot 
>> wrote:
>> > > >
>> > > > > Hi all,
>> > > > >
>> > > > > I would like to start a vote on KIP-559: Make the Kafka Protocol
>> > > Friendlier
>> > > > > with L7 Proxies.
>> > > > >
>> > > > > The KIP is here:
>> > > > >
>> > > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-559%3A+Make+the+Kafka+Protocol+Friendlier+with+L7+Proxies
>> > > > >
>> > > > > Thanks,
>> > > > > David
>> > > > >
>> > >
>> >
>> >
>> > --
>> > -- Guozhang
>> >
>>
>
>
> --
> -- Guozhang
>


-- 
-- Guozhang

Re: [DISCUSS] : KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[Navinder Brar]

Hi John,

Thanks for the responses. I will make the below changes as I had suggested 
earlier, and then close the vote in a few hours.

includeStaleStores -> staleStores
withIncludeStaleStores() > enableStaleStores()
includeStaleStores() -> staleStoresEnabled()

Thanks,
Navinder

Sent from Yahoo Mail for iPhone


On Friday, January 24, 2020, 5:36 AM, John Roesler  wrote:

Hi Bruno,

Thanks for your question; it's a very reasonable response to 
what I said before.

I didn't mean "field" as in an instance variable, just as in a specific
property or attribute. It's hard to talk about because all the words
for this abstract concept are also words that people commonly use
for instance variables.

The principle is that these classes should be simple data containers.
It's not so much that the methods match the field name, or that the
field name matches the methods, but that all three bear a simple
and direct relationship to each other. Or maybe I should say that
the getters, setters, and fields are all directly named after a property.

The point is that we should make sure we're not "playing games" in
these classes but simply setting a property and offering a transparent
way to get exactly what you just set.

I actually do think that the instance variable itself should have the
same name as the "field" or "property" that the getters and setters
are named for. This is not a violation of encapsulation because those 
instance variables are required to be private. 

 I guess you can think of  this rule as more of a style guide than a 
grammar, but whatever. As a maintainer, I think we should discourage 
these particular classes to have different instance variables than 
method names. Otherwise,  it's just silly. Either "includeStaleStores" 
or "staleStoresEnabled" is a fine name, but not both. There's no 
reason at all to name all the accessors one of them and the instance 
variable they access the  other name.

Thanks,
-John


On Thu, Jan 23, 2020, at 17:27, Bruno Cadonna wrote:
> Hi John,
> 
> One question: Why must the field name be involved in the naming? It
> somehow contradicts encapsulation. Field name `includeStaleStores` (or
> `staleStoresEnabled`) sounds perfectly fine to me. IMO, we should
> decouple the parameter name from the actual field name.
> 
> Bruno
> 
> On Thu, Jan 23, 2020 at 3:02 PM John Roesler  wrote:
> >
> > Hi all,
> >
> > Thanks for the discussion!
> >
> > The basic idea I used in the original draft of the grammar was to avoid
> > "fancy code" and just write "normal java". That's why I favored java bean
> > spec over Kafka code traditions.
> >
> > According to the bean spec, setters always start with "set" and getters
> > always start with "get" (or "is" for booleans). This often yields absurd
> > or awkward readability. On the other hand, the "kafka" idioms
> > seems to descend from Scala idiom of naming getters and setters
> > exactly the same as the field they get and set. This plays to a language
> > feature of Scala (getter referential transparency) that is not present
> > in Java. My feeling is that we probably keep this idiom around
> > precisely to avoid the absurd phrasing that the bean spec leads to.
> >
> > On the other hand, adopting the Kafka/Scala idiom brings in an
> > additional burden I was trying to avoid: you have to pick a good
> > name. Basically I was trying to avoid exactly this conversation ;)
> > i.e., "X sounds weird, how about Y", "well, Y also sounds weird,
> > what about Z", "Z sounds good, but then the setter sounds weird",
> > etc.
> >
> > Maybe avoiding discussion was too ambitious, and I can't deny that
> > bean spec names probably result in no one being happy, so I'm on
> > board with the current proposal:
> >
> > setters:
> > set{FieldName}(value)
> > {enable/disable}{FieldName}()
> >
> > getters:
> > {fieldName}()
> > {fieldName}{Enabled/Disabled}()
> >
> > Probably, we'll find cases that are silly under that formula too,
> > but we'll cross that bridge when we get to it.
> >
> > I'll update the grammar when I get the chance.
> >
> > Thanks!
> > -John
> >
> > On Thu, Jan 23, 2020, at 12:37, Navinder Brar wrote:
> > > Thanks Bruno, for the comments.
> > > 1) Fixed.
> > >
> > > 2) I would be okay to call the variable staleStores. Since anyways we
> > > are not using constructor, so the only way the variable is exposed
> > > outside is the getter and the optional builder method. With this
> > > variable name, we can name the builder method as "enableStaleStores"
> > > and I feel staleStoresEnabled() is more readable for getter function.
> > > So, we can also change the grammar for getters for boolean variables to
> > > {FlagName}enabled / {FlagName}disabled. WDYT @John.
> > >
> > > Thanks,
> > > Navinder  On Thursday, 23 January, 2020, 11:43:38 pm IST, Bruno
> > > Cadonna  wrote:
> > >
> > >  Hi Navinder,
> > >
> > > Thank you for the KIP!
> > >
> > > It looks good to me. Here my comments:
> > >
> > > 1) I agree with John and Matthias that you should remove 

Re: [DISCUSS] : KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[John Roesler]

Hi Bruno,

Thanks for your question; it's a very reasonable response to 
what I said before.

I didn't mean "field" as in an instance variable, just as in a specific
property or attribute. It's hard to talk about because all the words
for this abstract concept are also words that people commonly use
for instance variables.

The principle is that these classes should be simple data containers.
It's not so much that the methods match the field name, or that the
field name matches the methods, but that all three bear a simple
and direct relationship to each other. Or maybe I should say that
the getters, setters, and fields are all directly named after a property.

The point is that we should make sure we're not "playing games" in
these classes but simply setting a property and offering a transparent
way to get exactly what you just set.

I actually do think that the instance variable itself should have the
same name as the "field" or "property" that the getters and setters
are named for. This is not a violation of encapsulation because those 
instance variables are required to be private. 

 I guess you can think of  this rule as more of a style guide than a 
grammar, but whatever. As a maintainer, I think we should discourage 
these particular classes to have different instance variables than 
method names. Otherwise,  it's just silly. Either "includeStaleStores" 
or "staleStoresEnabled" is a fine name, but not both. There's no 
reason at all to name all the accessors one of them and the instance 
variable they access the  other name.

Thanks,
-John


On Thu, Jan 23, 2020, at 17:27, Bruno Cadonna wrote:
> Hi John,
> 
> One question: Why must the field name be involved in the naming? It
> somehow contradicts encapsulation. Field name `includeStaleStores` (or
> `staleStoresEnabled`) sounds perfectly fine to me. IMO, we should
> decouple the parameter name from the actual field name.
> 
> Bruno
> 
> On Thu, Jan 23, 2020 at 3:02 PM John Roesler  wrote:
> >
> > Hi all,
> >
> > Thanks for the discussion!
> >
> > The basic idea I used in the original draft of the grammar was to avoid
> > "fancy code" and just write "normal java". That's why I favored java bean
> > spec over Kafka code traditions.
> >
> > According to the bean spec, setters always start with "set" and getters
> > always start with "get" (or "is" for booleans). This often yields absurd
> > or awkward readability. On the other hand, the "kafka" idioms
> > seems to descend from Scala idiom of naming getters and setters
> > exactly the same as the field they get and set. This plays to a language
> > feature of Scala (getter referential transparency) that is not present
> > in Java. My feeling is that we probably keep this idiom around
> > precisely to avoid the absurd phrasing that the bean spec leads to.
> >
> > On the other hand, adopting the Kafka/Scala idiom brings in an
> > additional burden I was trying to avoid: you have to pick a good
> > name. Basically I was trying to avoid exactly this conversation ;)
> > i.e., "X sounds weird, how about Y", "well, Y also sounds weird,
> > what about Z", "Z sounds good, but then the setter sounds weird",
> > etc.
> >
> > Maybe avoiding discussion was too ambitious, and I can't deny that
> > bean spec names probably result in no one being happy, so I'm on
> > board with the current proposal:
> >
> > setters:
> > set{FieldName}(value)
> > {enable/disable}{FieldName}()
> >
> > getters:
> > {fieldName}()
> > {fieldName}{Enabled/Disabled}()
> >
> > Probably, we'll find cases that are silly under that formula too,
> > but we'll cross that bridge when we get to it.
> >
> > I'll update the grammar when I get the chance.
> >
> > Thanks!
> > -John
> >
> > On Thu, Jan 23, 2020, at 12:37, Navinder Brar wrote:
> > > Thanks Bruno, for the comments.
> > > 1) Fixed.
> > >
> > > 2) I would be okay to call the variable staleStores. Since anyways we
> > > are not using constructor, so the only way the variable is exposed
> > > outside is the getter and the optional builder method. With this
> > > variable name, we can name the builder method as "enableStaleStores"
> > > and I feel staleStoresEnabled() is more readable for getter function.
> > > So, we can also change the grammar for getters for boolean variables to
> > > {FlagName}enabled / {FlagName}disabled. WDYT @John.
> > >
> > > Thanks,
> > > Navinder   On Thursday, 23 January, 2020, 11:43:38 pm IST, Bruno
> > > Cadonna  wrote:
> > >
> > >  Hi Navinder,
> > >
> > > Thank you for the KIP!
> > >
> > > It looks good to me. Here my comments:
> > >
> > > 1) I agree with John and Matthias that you should remove the
> > > implementation of the methods in the KIP. Just the method signatures
> > > suffice and make the reading easier.
> > >
> > > 2) According to the grammar `withIncludeStaleStores()` should be
> > > `enableIncludeStaleStores()` but since that reads a bit clumsy I
> > > propose to call the method `enableStaleStores()`.
> > >
> > > 3) The getter 

Re: [DISCUSS] : KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[Bruno Cadonna]

Hi John,

One question: Why must the field name be involved in the naming? It
somehow contradicts encapsulation. Field name `includeStaleStores` (or
`staleStoresEnabled`) sounds perfectly fine to me. IMO, we should
decouple the parameter name from the actual field name.

Bruno

On Thu, Jan 23, 2020 at 3:02 PM John Roesler  wrote:
>
> Hi all,
>
> Thanks for the discussion!
>
> The basic idea I used in the original draft of the grammar was to avoid
> "fancy code" and just write "normal java". That's why I favored java bean
> spec over Kafka code traditions.
>
> According to the bean spec, setters always start with "set" and getters
> always start with "get" (or "is" for booleans). This often yields absurd
> or awkward readability. On the other hand, the "kafka" idioms
> seems to descend from Scala idiom of naming getters and setters
> exactly the same as the field they get and set. This plays to a language
> feature of Scala (getter referential transparency) that is not present
> in Java. My feeling is that we probably keep this idiom around
> precisely to avoid the absurd phrasing that the bean spec leads to.
>
> On the other hand, adopting the Kafka/Scala idiom brings in an
> additional burden I was trying to avoid: you have to pick a good
> name. Basically I was trying to avoid exactly this conversation ;)
> i.e., "X sounds weird, how about Y", "well, Y also sounds weird,
> what about Z", "Z sounds good, but then the setter sounds weird",
> etc.
>
> Maybe avoiding discussion was too ambitious, and I can't deny that
> bean spec names probably result in no one being happy, so I'm on
> board with the current proposal:
>
> setters:
> set{FieldName}(value)
> {enable/disable}{FieldName}()
>
> getters:
> {fieldName}()
> {fieldName}{Enabled/Disabled}()
>
> Probably, we'll find cases that are silly under that formula too,
> but we'll cross that bridge when we get to it.
>
> I'll update the grammar when I get the chance.
>
> Thanks!
> -John
>
> On Thu, Jan 23, 2020, at 12:37, Navinder Brar wrote:
> > Thanks Bruno, for the comments.
> > 1) Fixed.
> >
> > 2) I would be okay to call the variable staleStores. Since anyways we
> > are not using constructor, so the only way the variable is exposed
> > outside is the getter and the optional builder method. With this
> > variable name, we can name the builder method as "enableStaleStores"
> > and I feel staleStoresEnabled() is more readable for getter function.
> > So, we can also change the grammar for getters for boolean variables to
> > {FlagName}enabled / {FlagName}disabled. WDYT @John.
> >
> > Thanks,
> > Navinder   On Thursday, 23 January, 2020, 11:43:38 pm IST, Bruno
> > Cadonna  wrote:
> >
> >  Hi Navinder,
> >
> > Thank you for the KIP!
> >
> > It looks good to me. Here my comments:
> >
> > 1) I agree with John and Matthias that you should remove the
> > implementation of the methods in the KIP. Just the method signatures
> > suffice and make the reading easier.
> >
> > 2) According to the grammar `withIncludeStaleStores()` should be
> > `enableIncludeStaleStores()` but since that reads a bit clumsy I
> > propose to call the method `enableStaleStores()`.
> >
> > 3) The getter `includeStaleStores()` does not sound right to me. It
> > does not include stale stores but rather checks if stale stores should
> > be queried. Thus, I would call it `staleStoresEnabled()` (or
> > `staleStoresIncluded` but that does not align nicely with
> > `enableStaleStores()`). No need to change the field name, though.
> > Maybe, we could also add this special rule for getters of boolean
> > values to the grammar. WDYT?
> >
> > I have a final remark about the `StoreQueryParams`. I think it should
> > be immutable. That is more an implementation detail and we should
> > discuss it on the PR. Just wanted to mention it in advance. Probably
> > we should add also a rule for immutability to the grammar.
> >
> > Best,
> > Bruno
> >
> > On Wed, Jan 22, 2020 at 7:38 PM Navinder Brar
> >  wrote:
> > >
> > > +1 on changing to storeName() and includeStateStores(). We can add this 
> > > to grammar wiki as Matthias suggested.
> > >
> > > I have edited the KIP to remove "Deprecating" in favor of "Changing" and 
> > > I agree we can deprecate store(final String storeName, final 
> > > QueryableStoreType queryableStoreType).
> > >
> > > Thanks
> > > Navinder
> > >On Thursday, 23 January, 2020, 07:28:38 am IST, Matthias J. Sax 
> > >  wrote:
> > >
> > >  Thanks for the clarifications about the getters. I agree that it makes
> > > sense to move to the new pattern incrementally. Might be useful to
> > > create a Jira (or multiple?) to track this. It's an straight forward 
> > > change.
> > >
> > > A nit about the KIP: it should only list the signature but not the full
> > > code of the implementation (ie, only package name and the class + method
> > > names; we can omit toString(), equals(), and hashCode(), too -- alo, no
> > > license header please ;))
> > >
> > >
> > > nit: `isIncludeStaleStores` 

Re: [DISCUSS] : KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[John Roesler]

Hi all,

Thanks for the discussion!

The basic idea I used in the original draft of the grammar was to avoid
"fancy code" and just write "normal java". That's why I favored java bean
spec over Kafka code traditions.

According to the bean spec, setters always start with "set" and getters
always start with "get" (or "is" for booleans). This often yields absurd
or awkward readability. On the other hand, the "kafka" idioms
seems to descend from Scala idiom of naming getters and setters
exactly the same as the field they get and set. This plays to a language
feature of Scala (getter referential transparency) that is not present 
in Java. My feeling is that we probably keep this idiom around 
precisely to avoid the absurd phrasing that the bean spec leads to.

On the other hand, adopting the Kafka/Scala idiom brings in an
additional burden I was trying to avoid: you have to pick a good
name. Basically I was trying to avoid exactly this conversation ;)
i.e., "X sounds weird, how about Y", "well, Y also sounds weird,
what about Z", "Z sounds good, but then the setter sounds weird",
etc.

Maybe avoiding discussion was too ambitious, and I can't deny that
bean spec names probably result in no one being happy, so I'm on
board with the current proposal:

setters:
set{FieldName}(value)
{enable/disable}{FieldName}()

getters:
{fieldName}()
{fieldName}{Enabled/Disabled}()

Probably, we'll find cases that are silly under that formula too,
but we'll cross that bridge when we get to it.

I'll update the grammar when I get the chance.

Thanks!
-John

On Thu, Jan 23, 2020, at 12:37, Navinder Brar wrote:
> Thanks Bruno, for the comments.
> 1) Fixed.
> 
> 2) I would be okay to call the variable staleStores. Since anyways we 
> are not using constructor, so the only way the variable is exposed 
> outside is the getter and the optional builder method. With this 
> variable name, we can name the builder method as "enableStaleStores" 
> and I feel staleStoresEnabled() is more readable for getter function. 
> So, we can also change the grammar for getters for boolean variables to 
> {FlagName}enabled / {FlagName}disabled. WDYT @John.
> 
> Thanks,
> Navinder   On Thursday, 23 January, 2020, 11:43:38 pm IST, Bruno 
> Cadonna  wrote:  
>  
>  Hi Navinder,
> 
> Thank you for the KIP!
> 
> It looks good to me. Here my comments:
> 
> 1) I agree with John and Matthias that you should remove the
> implementation of the methods in the KIP. Just the method signatures
> suffice and make the reading easier.
> 
> 2) According to the grammar `withIncludeStaleStores()` should be
> `enableIncludeStaleStores()` but since that reads a bit clumsy I
> propose to call the method `enableStaleStores()`.
> 
> 3) The getter `includeStaleStores()` does not sound right to me. It
> does not include stale stores but rather checks if stale stores should
> be queried. Thus, I would call it `staleStoresEnabled()` (or
> `staleStoresIncluded` but that does not align nicely with
> `enableStaleStores()`). No need to change the field name, though.
> Maybe, we could also add this special rule for getters of boolean
> values to the grammar. WDYT?
> 
> I have a final remark about the `StoreQueryParams`. I think it should
> be immutable. That is more an implementation detail and we should
> discuss it on the PR. Just wanted to mention it in advance. Probably
> we should add also a rule for immutability to the grammar.
> 
> Best,
> Bruno
> 
> On Wed, Jan 22, 2020 at 7:38 PM Navinder Brar
>  wrote:
> >
> > +1 on changing to storeName() and includeStateStores(). We can add this to 
> > grammar wiki as Matthias suggested.
> >
> > I have edited the KIP to remove "Deprecating" in favor of "Changing" and I 
> > agree we can deprecate store(final String storeName, final 
> > QueryableStoreType queryableStoreType).
> >
> > Thanks
> > Navinder
> >    On Thursday, 23 January, 2020, 07:28:38 am IST, Matthias J. Sax 
> > wrote:
> >
> >  Thanks for the clarifications about the getters. I agree that it makes
> > sense to move to the new pattern incrementally. Might be useful to
> > create a Jira (or multiple?) to track this. It's an straight forward change.
> >
> > A nit about the KIP: it should only list the signature but not the full
> > code of the implementation (ie, only package name and the class + method
> > names; we can omit toString(), equals(), and hashCode(), too -- alo, no
> > license header please ;))
> >
> >
> > nit: `isIncludeStaleStores` -> `includeStaleStores` (the "is"-prefix
> > reads clumsy and it's common in Kafka code base to omit the "get"-prefix
> > for getters -- we should adopt this)
> >
> > @John: might be worth to include this in the Grammar wiki page?
> >
> > nit (similar as above):
> >
> >  - `getStoreName` -> `storeName`
> >  - `getQueryableStoreType` -> `queryableStoreType`
> >
> >
> > The KIP says
> >
> > > Deprecating the KafkaStreams#store(final String storeName, final 
> > > QueryableStoreType queryableStoreType, final boolean 
> > > 

[jira] [Resolved] (KAFKA-8317) ClassCastException using KTable.suppress()

[Matthias J. Sax (Jira)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-8317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Matthias J. Sax resolved KAFKA-8317.

Resolution: Duplicate

Closing this ticket in favor of KAFKA-9259 that duplicates it.

> ClassCastException using KTable.suppress()
> --
>
> Key: KAFKA-8317
> URL: https://issues.apache.org/jira/browse/KAFKA-8317
> Project: Kafka
>  Issue Type: Bug
>Reporter: Andrew
>Priority: Major
>
> I am trying to use `KTable.suppress()` and I am getting the following error :
> {Code}
> java.lang.ClassCastException: org.apache.kafka.streams.kstream.Windowed 
> cannot be cast to java.lang.String
>     at 
> org.apache.kafka.common.serialization.StringSerializer.serialize(StringSerializer.java:28)
>     at 
> org.apache.kafka.streams.kstream.internals.suppress.KTableSuppressProcessor.buffer(KTableSuppressProcessor.java:95)
>     at 
> org.apache.kafka.streams.kstream.internals.suppress.KTableSuppressProcessor.process(KTableSuppressProcessor.java:87)
>     at 
> org.apache.kafka.streams.kstream.internals.suppress.KTableSuppressProcessor.process(KTableSuppressProcessor.java:40)
>     at 
> org.apache.kafka.streams.processor.internals.ProcessorNode.process(ProcessorNode.java:117)
> {Code}
> My code is as follows :
> {Code}
>     final KTable, GenericRecord> groupTable = 
> groupedStream
>     .aggregate(lastAggregator, lastAggregator, materialized);
>     final KTable, GenericRecord> suppressedTable = 
> groupTable.suppress(Suppressed.untilWindowCloses(Suppressed.BufferConfig.unbounded()));
>     // write the change-log stream to the topic
>     suppressedTable.toStream((k, v) -> k.key())
>     .mapValues(joinValueMapper::apply)
>     .to(props.joinTopic());
> {Code}
> The code without using `suppressedTable` works... what am i doing wrong.
> Someone else has encountered the same issue :
> https://gist.github.com/robie2011/1caa4772b60b5a6f993e6f98e792a380
> Slack conversation : 
> https://confluentcommunity.slack.com/archives/C48AHTCUQ/p1556633088239800



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Re: [VOTE] KIP-559: Make the Kafka Protocol Friendlier with L7 Proxies

[Guozhang Wang]

Yeah that makes sense, it is a good-to-have if we can push through this in
2.5 but if we do not have bandwidth that's fine too :)

Guozhang

On Thu, Jan 23, 2020 at 1:40 PM David Jacot  wrote:

> Hi Guozhang,
>
> Thank you for your input.
>
> 1) You're right. I've put it there due to the version bump only. I'll make
> it clearer.
>
> 2) I'd rather prefer to keep the scope as it is because 1) that field is
> not related to
> the problem that we are solving here and 2) I am not sure that I will have
> the
> bandwidth to do this before the feature freeze. The PR is already ready.
> That being
> said, as the addition of that field is part of KIP-429 and KIP-429 has
> already been
> accepted, we could give it a shot to avoid having to bump the version
> twice. I could
> try putting together a PR before the feature freeze but without guarantee.
> Does that
> make sense?
>
> David
>
> On Thu, Jan 23, 2020 at 9:44 AM Guozhang Wang  wrote:
>
> > Hello David,
> >
> > Thanks for the KIP! I have read through the proposal and had one minor
> and
> > one meta comment. But overall it looks good to me!
> >
> > 1) The JoinGroupRequest format does not have any new fields proposed, so
> we
> > could either clarify that it is listed here but without modifications
> (only
> > version bumps) or just remove it from the wiki.
> >
> > 2) Could we consider adding a "protocol version" to allow brokers to
> select
> > the leader with the highest version? This thought is brought up in
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-429%3A+Kafka+Consumer+Incremental+Rebalance+Protocol#KIP-429:KafkaConsumerIncrementalRebalanceProtocol-LookingintotheFuture:AssignorVersion
> > .
> > I'm fine with keeping this KIP's scope as is, just wondering if you feel
> > comfortable piggy-backing this change as well if we are going to bump up
> > the JoinGroupReq/Response anyways.
> >
> >
> > Guozhang
> >
> >
> > On Wed, Jan 22, 2020 at 9:10 AM Eno Thereska 
> > wrote:
> >
> > > This is awesome! +1 (non binding)
> > > Eno
> > >
> > > On Tue, Jan 21, 2020 at 10:00 PM Gwen Shapira 
> wrote:
> > > >
> > > > Thank you for the KIP. Awesomely cloud-native improvement :)
> > > >
> > > > +1 (binding)
> > > >
> > > >
> > > > On Tue, Jan 21, 2020, 9:35 AM David Jacot 
> wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I would like to start a vote on KIP-559: Make the Kafka Protocol
> > > Friendlier
> > > > > with L7 Proxies.
> > > > >
> > > > > The KIP is here:
> > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-559%3A+Make+the+Kafka+Protocol+Friendlier+with+L7+Proxies
> > > > >
> > > > > Thanks,
> > > > > David
> > > > >
> > >
> >
> >
> > --
> > -- Guozhang
> >
>


-- 
-- Guozhang

Re: [VOTE] KIP-559: Make the Kafka Protocol Friendlier with L7 Proxies

[David Jacot]

Hi Guozhang,

Thank you for your input.

1) You're right. I've put it there due to the version bump only. I'll make
it clearer.

2) I'd rather prefer to keep the scope as it is because 1) that field is
not related to
the problem that we are solving here and 2) I am not sure that I will have
the
bandwidth to do this before the feature freeze. The PR is already ready.
That being
said, as the addition of that field is part of KIP-429 and KIP-429 has
already been
accepted, we could give it a shot to avoid having to bump the version
twice. I could
try putting together a PR before the feature freeze but without guarantee.
Does that
make sense?

David

On Thu, Jan 23, 2020 at 9:44 AM Guozhang Wang  wrote:

> Hello David,
>
> Thanks for the KIP! I have read through the proposal and had one minor and
> one meta comment. But overall it looks good to me!
>
> 1) The JoinGroupRequest format does not have any new fields proposed, so we
> could either clarify that it is listed here but without modifications (only
> version bumps) or just remove it from the wiki.
>
> 2) Could we consider adding a "protocol version" to allow brokers to select
> the leader with the highest version? This thought is brought up in
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-429%3A+Kafka+Consumer+Incremental+Rebalance+Protocol#KIP-429:KafkaConsumerIncrementalRebalanceProtocol-LookingintotheFuture:AssignorVersion
> .
> I'm fine with keeping this KIP's scope as is, just wondering if you feel
> comfortable piggy-backing this change as well if we are going to bump up
> the JoinGroupReq/Response anyways.
>
>
> Guozhang
>
>
> On Wed, Jan 22, 2020 at 9:10 AM Eno Thereska 
> wrote:
>
> > This is awesome! +1 (non binding)
> > Eno
> >
> > On Tue, Jan 21, 2020 at 10:00 PM Gwen Shapira  wrote:
> > >
> > > Thank you for the KIP. Awesomely cloud-native improvement :)
> > >
> > > +1 (binding)
> > >
> > >
> > > On Tue, Jan 21, 2020, 9:35 AM David Jacot  wrote:
> > >
> > > > Hi all,
> > > >
> > > > I would like to start a vote on KIP-559: Make the Kafka Protocol
> > Friendlier
> > > > with L7 Proxies.
> > > >
> > > > The KIP is here:
> > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-559%3A+Make+the+Kafka+Protocol+Friendlier+with+L7+Proxies
> > > >
> > > > Thanks,
> > > > David
> > > >
> >
>
>
> --
> -- Guozhang
>

Re: [DISCUSS] KIP-566: Add rebalance callbacks to ConsumerInterceptor

[M. Manna]

Hey Thomas,

On Thu, 23 Jan 2020 at 21:17, Thomas Becker  wrote:

> Hi folks,
> I'd like to open the discussion for KIP-566: Add rebalance callbacks to
> ConsumerInterceptor. We've been looking to implement some custom metrics
> via ConsumerInterceptor, and not knowing when partition ownership changes
> is a significant impediment. I'd appreciate your thoughts.
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-566%3A+Add+rebalance+callbacks+to+ConsumerInterceptor
>
>
>
>  I had a quick read through the KIP. I don't see any obvious issues.
Sounds like a simple improvement. Perhaps, you could add your thoughts
about RebalanceListener API in the future e.g. when to unify the
functionality. If implemented, we can simply use one implementation for
both things.

I would be interested to hear others' comments about this.

Thanks,


>
> 
>
> This email and any attachments may contain confidential and privileged
> material for the sole use of the intended recipient. Any review, copying,
> or distribution of this email (or any attachments) by others is prohibited.
> If you are not the intended recipient, please contact the sender
> immediately and permanently delete this email and any attachments. No
> employee or agent of TiVo is authorized to conclude any binding agreement
> on behalf of TiVo by email. Binding agreements with TiVo may only be made
> by a signed written agreement.
>

[DISCUSS] KIP-566: Add rebalance callbacks to ConsumerInterceptor

[Thomas Becker]

Hi folks,
I'd like to open the discussion for KIP-566: Add rebalance callbacks to 
ConsumerInterceptor. We've been looking to implement some custom metrics via 
ConsumerInterceptor, and not knowing when partition ownership changes is a 
significant impediment. I'd appreciate your thoughts.

https://cwiki.apache.org/confluence/display/KAFKA/KIP-566%3A+Add+rebalance+callbacks+to+ConsumerInterceptor






This email and any attachments may contain confidential and privileged material 
for the sole use of the intended recipient. Any review, copying, or 
distribution of this email (or any attachments) by others is prohibited. If you 
are not the intended recipient, please contact the sender immediately and 
permanently delete this email and any attachments. No employee or agent of TiVo 
is authorized to conclude any binding agreement on behalf of TiVo by email. 
Binding agreements with TiVo may only be made by a signed written agreement.

Build failed in Jenkins: kafka-trunk-jdk11 #1104

[Apache Jenkins Server]

See 


Changes:

[github] KAFKA-7737; Use single path in producer for initializing the producerId


--
[...truncated 5.76 MB...]
org.apache.kafka.streams.TopologyTestDriverTest > shouldInitProcessor[Eos 
enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldThrowForUnknownTopic[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldThrowForUnknownTopic[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateOnStreamsTime[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateOnStreamsTime[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldThrowIfInMemoryBuiltInStoreIsAccessedWithUntypedMethod[Eos enabled = 
false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldThrowIfInMemoryBuiltInStoreIsAccessedWithUntypedMethod[Eos enabled = 
false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessFromSourcesThatMatchMultiplePattern[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessFromSourcesThatMatchMultiplePattern[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > shouldPopulateGlobalStore[Eos 
enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > shouldPopulateGlobalStore[Eos 
enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldThrowIfPersistentBuiltInStoreIsAccessedWithUntypedMethod[Eos enabled = 
false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldThrowIfPersistentBuiltInStoreIsAccessedWithUntypedMethod[Eos enabled = 
false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldAllowPrePopulatingStatesStoresWithCachingEnabled[Eos enabled = false] 
STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldAllowPrePopulatingStatesStoresWithCachingEnabled[Eos enabled = false] 
PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldReturnCorrectPersistentStoreTypeOnly[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldReturnCorrectPersistentStoreTypeOnly[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldUseSourceSpecificDeserializers[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldUseSourceSpecificDeserializers[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > shouldReturnAllStores[Eos 
enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > shouldReturnAllStores[Eos 
enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldNotCreateStateDirectoryForStatelessTopology[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldNotCreateStateDirectoryForStatelessTopology[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldReturnAllStoresNames[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldReturnAllStoresNames[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessConsumerRecordList[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessConsumerRecordList[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldUseSinkSpecificSerializers[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldUseSinkSpecificSerializers[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldFlushStoreForFirstInput[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldFlushStoreForFirstInput[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessFromSourceThatMatchPattern[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldProcessFromSourceThatMatchPattern[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldUpdateStoreForNewKey[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldUpdateStoreForNewKey[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldSendRecordViaCorrectSourceTopicDeprecated[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldSendRecordViaCorrectSourceTopicDeprecated[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateOnWallClockTime[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateOnWallClockTime[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > shouldSetRecordMetadata[Eos 
enabled = false] STARTED

Re: [VOTE] KIP-526: Reduce Producer Metadata Lookups for Large Number of Topics

[Jason Gustafson]

Sounds good. +1 from me.

On Thu, Jan 23, 2020 at 9:00 AM Brian Byrne  wrote:

> Thanks Jason,
>
> I'm in favor of the latter: metadata.max.idle.ms. I agree that describing
> it as a "period" is inaccurate. With metadata.max.idle.ms, it also aligns
> with metadata.max.age.ms for determining refresh period (which is an
> actual
> period).
>
> I've updated the docs.
>
> Thanks,
> Brian
>
> On Wed, Jan 22, 2020 at 6:19 PM Jason Gustafson 
> wrote:
>
> > Thanks for the proposal. Looks good overall. I wanted to suggest a
> possible
> > name change. I was considering something like `
> idle.metadata.expiration.ms
> > `
> > or maybe `metadata.max.idle.ms`. Thoughts?
> >
> > -Jason
> >
> >
> > On Tue, Jan 21, 2020 at 11:38 AM Guozhang Wang 
> wrote:
> >
> > > Got it.
> > >
> > > I was proposing that we do the "delayed async batch" but I think your
> > > argument for complexity and pushing it out of the scope is convincing,
> so
> > > instead I propose we do the synchronous mini batching still but
> obviously
> > > it is already there :)  I'm +1 on the current proposal scope.
> > >
> > > Guozhang
> > >
> > > On Tue, Jan 21, 2020 at 10:16 AM Brian Byrne 
> > wrote:
> > >
> > > > Hi Guozhang,
> > > >
> > > > Ah, sorry, I misunderstood. Actually, this is solved for us today.
> How
> > > the
> > > > producer works is that it maintains at most one inflight metadata
> fetch
> > > > request at any time, where each request is tagged with the current
> > > > (monotonically increasing) request version. This version is bumped
> > > whenever
> > > > a new topic is encountered, and metadata fetching will continue to
> > > process
> > > > while the latest metadata response's version is below the current
> > > version.
> > > >
> > > > So if a metadata request is in flight, and a number of threads
> produce
> > to
> > > > new topics, they'll be added to the working set but the next metadata
> > > > request won't take place until the outstanding one returns. So their
> > > > updates will be batched together. As you suggest, we can have a
> simple
> > > list
> > > > that tracks unknown topics to isolate new vs. old topics.
> > > >
> > > > Thanks,
> > > > Brian
> > > >
> > > >
> > > >
> > > > On Tue, Jan 21, 2020 at 10:04 AM Guozhang Wang 
> > > wrote:
> > > >
> > > > > Hi Brian,
> > > > >
> > > > > I think I buy the complexity and extra end-to-end-latency argument
> :)
> > > I'm
> > > > > fine with delaying the asynchronous tech fetching to future works
> and
> > > > keep
> > > > > the current KIP's scope as-is for now. Under that case can we
> > consider
> > > > just
> > > > > a minor implementation detail (since it is not affecting public
> APIs
> > we
> > > > > probably do not even need to list it, but just thinking loud here):
> > > > >
> > > > > In your proposal when we request for a topic of unknown metadata,
> we
> > > are
> > > > > going to directly set the topic name as that singleton in the
> > request.
> > > > I'm
> > > > > wondering for the scenario that KAFKA-8904 described, if the
> > > > producer#send
> > > > > for thousands of new topics are triggered sequentially by a single
> > > thread
> > > > > or concurrent threads? If it's the latter, and we expect in such
> > > > scenarios
> > > > > we may have multiple topics being requests within a very short
> time,
> > > then
> > > > > we can probably do sth. like this internally in a synchronized
> > manner:
> > > > >
> > > > > 1) put the topic name into a list, as "unknown topics", then
> > > > > 2) exhaust the list, and put all topics from that list to the
> > request;
> > > if
> > > > > the list is empty, it means it has been emptied by another thread
> so
> > we
> > > > > skip sending a new request and just wait for the returned metadata
> > > > refresh.
> > > > >
> > > > > In most cases the list would just be a singleton with the one that
> > > thread
> > > > > has just enqueued, but under extreme scenarios it can help
> batching a
> > > few
> > > > > topic names probably (of course, I'm thinking about very extreme
> > cases
> > > > > here, assuming that's was what we've seen in 8904). Since these two
> > > steps
> > > > > are very light-weighted, doing that in a synchronized block would
> not
> > > > hurt
> > > > > the concurrency too much.
> > > > >
> > > > >
> > > > > Guozhang
> > > > >
> > > > >
> > > > > On Tue, Jan 21, 2020 at 9:39 AM Brian Byrne 
> > > wrote:
> > > > >
> > > > > > Hi Guozhang,
> > > > > >
> > > > > > Your understanding of the rationale is accurate, and what you
> > suggest
> > > > is
> > > > > > completely plausible, however I have a slightly different take on
> > the
> > > > > > situation.
> > > > > >
> > > > > > When the KIP was originally drafted, making KafkaProducer#send
> > > > > asynchronous
> > > > > > was one element of the changes (this is a little more general
> than
> > > (a),
> > > > > but
> > > > > > has similar implications). As you're aware, doing so would allow
> > new
> > > > > topics
> > > > > > to aggregate since the 

[jira] [Created] (KAFKA-9470) Hanging test case `testBlockOnRequestCompletionFromStateChangeHandler`

[Jason Gustafson (Jira)]

Jason Gustafson created KAFKA-9470:
--

 Summary: Hanging test case 
`testBlockOnRequestCompletionFromStateChangeHandler`
 Key: KAFKA-9470
 URL: https://issues.apache.org/jira/browse/KAFKA-9470
 Project: Kafka
  Issue Type: Bug
Reporter: Jason Gustafson


I have seen several jenkins builds timeout due to `ZookeeperClientTest. 
testBlockOnRequestCompletionFromStateChangeHandler`. 

>From a recent jenkins build:
{code}
17:05:31 kafka.zookeeper.ZooKeeperClientTest > 
testBlockOnRequestCompletionFromStateChangeHandler STARTED
...
20:51:47 kafka.zookeeper.ZooKeeperClientTest > 
testBlockOnRequestCompletionFromStateChangeHandler SKIPPED
{code}

No additional information is available from the build and I've been unable to 
reproduce this. It could be related to KAFKA-8532.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Created] (KAFKA-9472) Reducing number of tasks for connector causes deleted tasks to show as UNASSIGNED

[Chris Egerton (Jira)]

Chris Egerton created KAFKA-9472:


 Summary: Reducing number of tasks for connector causes deleted 
tasks to show as UNASSIGNED
 Key: KAFKA-9472
 URL: https://issues.apache.org/jira/browse/KAFKA-9472
 Project: Kafka
  Issue Type: Bug
  Components: KafkaConnect
Affects Versions: 2.3.1, 2.4.0, 2.2.2, 2.2.1, 2.3.0, 2.1.1, 2.2.0, 2.1.0, 
2.0.1, 2.0.0
Reporter: Chris Egerton


If a connector is successfully created with {{t1}} running tasks and then 
reconfigured to use {{t1 - n}} tasks (where {{t1}} and {{n}} are both whole 
numbers and {{n}} is strictly less than {{t1}}), the connector should then list 
{{t1 - n}} total tasks in its status (which can be queried via the 
{{/connectors/:name:/status}} endpoint or the {{/connectors}} endpoint with the 
{{expand}} URL query parameter set to {{status}}).

However, the connector will instead continue to list {{t1}} total tasks in its 
status, with {{n}} of them being listed as {{UNASSIGNED}} and the remaining 
{{t1 - n}} of them being listed as {{STARTED}}.

This is because the only time a task status is removed from the status backing 
store (as opposed to simply being updated to {{UNASSIGNED}}) is when its 
connector is deleted. See relevant code snippets from the 
[AbstractHerder|https://github.com/apache/kafka/blob/df13fc93d0aebfe0ecc40dd4af3c5fb19b35f710/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java#L187-L192]
 and 
[DistributedHerder|https://github.com/apache/kafka/blob/df13fc93d0aebfe0ecc40dd4af3c5fb19b35f710/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java#L1511-L1520]
 classes.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Created] (KAFKA-9471) Return empty collection for PENDING_SHUTDOWN

[Ted Yu (Jira)]

Ted Yu created KAFKA-9471:
-

 Summary: Return empty collection for PENDING_SHUTDOWN
 Key: KAFKA-9471
 URL: https://issues.apache.org/jira/browse/KAFKA-9471
 Project: Kafka
  Issue Type: Improvement
Reporter: Ted Yu
Assignee: Ted Yu


In StreamThreadStateStoreProvider we have:
{code}
if (streamThread.state() == StreamThread.State.DEAD) {
return Collections.emptyList();
{code}
PENDING_SHUTDOWN should be treated the same way as DEAD.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Re: [DISCUSS] : KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[Bruno Cadonna]

Hi Navinder,

Thank you for the KIP!

It looks good to me. Here my comments:

1) I agree with John and Matthias that you should remove the
implementation of the methods in the KIP. Just the method signatures
suffice and make the reading easier.

2) According to the grammar `withIncludeStaleStores()` should be
`enableIncludeStaleStores()` but since that reads a bit clumsy I
propose to call the method `enableStaleStores()`.

3) The getter `includeStaleStores()` does not sound right to me. It
does not include stale stores but rather checks if stale stores should
be queried. Thus, I would call it `staleStoresEnabled()` (or
`staleStoresIncluded` but that does not align nicely with
`enableStaleStores()`). No need to change the field name, though.
Maybe, we could also add this special rule for getters of boolean
values to the grammar. WDYT?

I have a final remark about the `StoreQueryParams`. I think it should
be immutable. That is more an implementation detail and we should
discuss it on the PR. Just wanted to mention it in advance. Probably
we should add also a rule for immutability to the grammar.

Best,
Bruno

On Wed, Jan 22, 2020 at 7:38 PM Navinder Brar
 wrote:
>
> +1 on changing to storeName() and includeStateStores(). We can add this to 
> grammar wiki as Matthias suggested.
>
> I have edited the KIP to remove "Deprecating" in favor of "Changing" and I 
> agree we can deprecate store(final String storeName, final 
> QueryableStoreType queryableStoreType).
>
> Thanks
> Navinder
> On Thursday, 23 January, 2020, 07:28:38 am IST, Matthias J. Sax 
>  wrote:
>
>  Thanks for the clarifications about the getters. I agree that it makes
> sense to move to the new pattern incrementally. Might be useful to
> create a Jira (or multiple?) to track this. It's an straight forward change.
>
> A nit about the KIP: it should only list the signature but not the full
> code of the implementation (ie, only package name and the class + method
> names; we can omit toString(), equals(), and hashCode(), too -- alo, no
> license header please ;))
>
>
> nit: `isIncludeStaleStores` -> `includeStaleStores` (the "is"-prefix
> reads clumsy and it's common in Kafka code base to omit the "get"-prefix
> for getters -- we should adopt this)
>
> @John: might be worth to include this in the Grammar wiki page?
>
> nit (similar as above):
>
>  - `getStoreName` -> `storeName`
>  - `getQueryableStoreType` -> `queryableStoreType`
>
>
> The KIP says
>
> > Deprecating the KafkaStreams#store(final String storeName, final 
> > QueryableStoreType queryableStoreType, final boolean includeStaleStores) 
> > in favour of the funtion mentioned below.
>
> We don't need to deprecate this method but we can remove it directly,
> because it was never release.
>
>
> What is the plan for
>
> > store(final String storeName, final QueryableStoreType 
> > queryableStoreType) {
>
> Given that the new `StoreQueryParams` allows to specify `storeName` and
> `queryableStoreType`, should we deprecate this method in favor of the
> new `store(StoreQueryParams)` overload?
>
>
> -Matthias
>
>
>
> On 1/22/20 10:06 AM, John Roesler wrote:
> > Thanks Navinder! I've also updated the motivation.
> >
> > Thanks,
> > -John
> >
> > On Wed, Jan 22, 2020, at 11:12, Navinder Brar wrote:
> >> I went through the grammar wiki page and since it is already agreed in
> >> principle I will change from constructor to below method and add the
> >> getters back.
> >> public static  StoreQueryParams fromNameAndType(
> >>   final String storeName,
> >>   final QueryableStoreType  queryableStoreType
> >> )
> >>
> >>
> >> Thanks,
> >> Navinder
> >>
> >>On Wednesday, 22 January, 2020, 09:32:07 pm IST, John Roesler
> >>  wrote:
> >>
> >>  22) I'm specifically proposing to establish a new convention.
> >> The existing convention is fundamentally broken and has
> >> been costly both for users and maintainers. That is the purpose
> >> of the grammar I proposed. The plan is to implement  new APIs
> >> following the grammar and gradually to port old APIs to it.
> >>
> >> The grammar wiki page has plenty of justification, so I won't
> >> recapitulate it here.
> >>
> >> Thanks,
> >> -John
> >>
> >> On Wed, Jan 22, 2020, at 09:39, Navinder Brar wrote:
> >>> 10) Sure John, please go ahead.
> >>>
> >>> 21) I have no strong opinion on constructor vs static factory. If
> >>> everyone's okay with it, I can make the change.
> >>>
> >>> 22) I looked at classes suggested by Matthias and I see there are no
> >>> getters there. We are ok with breaking the convention?
> >>>
> >>> Thanks,Navinder Pal Singh Brar
> >>>
> >>>
> >>>
> >>> On Wednesday, 22 January, 2020, 08:40:27 pm IST, John Roesler
> >>>  wrote:
> >>>
> >>>   Hi all,
> >>>
> >>> 10) For the motivation, I have some thoughts for why this KIP is
> >>> absolutely essential as designed. If it's ok with you, Navinder,
> >>> I'd just edit the motivation section of the wiki? If you're unhappy
> >>> with my wording, you're of course welcome to revert or 

Build failed in Jenkins: kafka-trunk-jdk8 #4180

[Apache Jenkins Server]

See 


Changes:

[github] KAFKA-7737; Use single path in producer for initializing the producerId


--
[...truncated 2.83 MB...]
org.apache.kafka.streams.TopologyTestDriverTest > 
shouldNotUpdateStoreForSmallerValue[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldCreateStateDirectoryForStatefulTopology[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldCreateStateDirectoryForStatefulTopology[Eos enabled = false] PASSED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateIfWallClockTimeAdvances[Eos enabled = false] STARTED

org.apache.kafka.streams.TopologyTestDriverTest > 
shouldPunctuateIfWallClockTimeAdvances[Eos enabled = false] PASSED

org.apache.kafka.streams.MockTimeTest > shouldGetNanosAsMillis STARTED

org.apache.kafka.streams.MockTimeTest > shouldGetNanosAsMillis PASSED

org.apache.kafka.streams.MockTimeTest > shouldSetStartTime STARTED

org.apache.kafka.streams.MockTimeTest > shouldSetStartTime PASSED

org.apache.kafka.streams.MockTimeTest > shouldNotAllowNegativeSleep STARTED

org.apache.kafka.streams.MockTimeTest > shouldNotAllowNegativeSleep PASSED

org.apache.kafka.streams.MockTimeTest > shouldAdvanceTimeOnSleep STARTED

org.apache.kafka.streams.MockTimeTest > shouldAdvanceTimeOnSleep PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnIsOpen 
STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnIsOpen 
PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnName 
STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldReturnName 
PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldPutWithUnknownTimestamp STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldPutWithUnknownTimestamp PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldPutWindowStartTimestampWithUnknownTimestamp STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldPutWindowStartTimestampWithUnknownTimestamp PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldReturnIsPersistent STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > 
shouldReturnIsPersistent PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldForwardClose 
STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldForwardClose 
PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldForwardFlush 
STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldForwardFlush 
PASSED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldForwardInit 
STARTED

org.apache.kafka.streams.internals.WindowStoreFacadeTest > shouldForwardInit 
PASSED

org.apache.kafka.streams.test.TestRecordTest > testConsumerRecord STARTED

org.apache.kafka.streams.test.TestRecordTest > testConsumerRecord PASSED

org.apache.kafka.streams.test.TestRecordTest > testToString STARTED

org.apache.kafka.streams.test.TestRecordTest > testToString PASSED

org.apache.kafka.streams.test.TestRecordTest > testInvalidRecords STARTED

org.apache.kafka.streams.test.TestRecordTest > testInvalidRecords PASSED

org.apache.kafka.streams.test.TestRecordTest > testPartialConstructorEquals 
STARTED

org.apache.kafka.streams.test.TestRecordTest > testPartialConstructorEquals 
PASSED

org.apache.kafka.streams.test.TestRecordTest > testMultiFieldMatcher STARTED

org.apache.kafka.streams.test.TestRecordTest > testMultiFieldMatcher PASSED

org.apache.kafka.streams.test.TestRecordTest > testFields STARTED

org.apache.kafka.streams.test.TestRecordTest > testFields PASSED

org.apache.kafka.streams.test.TestRecordTest > testProducerRecord STARTED

org.apache.kafka.streams.test.TestRecordTest > testProducerRecord PASSED

org.apache.kafka.streams.test.TestRecordTest > testEqualsAndHashCode STARTED

org.apache.kafka.streams.test.TestRecordTest > testEqualsAndHashCode PASSED

> Task :streams:upgrade-system-tests-0100:compileJava NO-SOURCE
> Task :streams:upgrade-system-tests-0100:processResources NO-SOURCE
> Task :streams:upgrade-system-tests-0100:classes UP-TO-DATE
> Task :streams:upgrade-system-tests-0100:checkstyleMain NO-SOURCE
> Task :streams:upgrade-system-tests-0100:compileTestJava
> Task :streams:upgrade-system-tests-0100:processTestResources NO-SOURCE
> Task :streams:upgrade-system-tests-0100:testClasses
> Task :streams:upgrade-system-tests-0100:checkstyleTest
> Task :streams:upgrade-system-tests-0100:spotbugsMain NO-SOURCE
> Task :streams:upgrade-system-tests-0100:test
> Task :streams:upgrade-system-tests-0101:compileJava NO-SOURCE
> Task :streams:upgrade-system-tests-0101:processResources NO-SOURCE
> Task :streams:upgrade-system-tests-0101:classes UP-TO-DATE
> Task :streams:upgrade-system-tests-0101:checkstyleMain 

Re: [DISCUSS] : KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[Navinder Brar]

Thanks Bruno, for the comments.
1) Fixed.

2) I would be okay to call the variable staleStores. Since anyways we are not 
using constructor, so the only way the variable is exposed outside is the 
getter and the optional builder method. With this variable name, we can name 
the builder method as "enableStaleStores" and I feel staleStoresEnabled() is 
more readable for getter function. So, we can also change the grammar for 
getters for boolean variables to {FlagName}enabled / {FlagName}disabled. WDYT 
@John.

Thanks,
Navinder   On Thursday, 23 January, 2020, 11:43:38 pm IST, Bruno Cadonna 
 wrote:  
 
 Hi Navinder,

Thank you for the KIP!

It looks good to me. Here my comments:

1) I agree with John and Matthias that you should remove the
implementation of the methods in the KIP. Just the method signatures
suffice and make the reading easier.

2) According to the grammar `withIncludeStaleStores()` should be
`enableIncludeStaleStores()` but since that reads a bit clumsy I
propose to call the method `enableStaleStores()`.

3) The getter `includeStaleStores()` does not sound right to me. It
does not include stale stores but rather checks if stale stores should
be queried. Thus, I would call it `staleStoresEnabled()` (or
`staleStoresIncluded` but that does not align nicely with
`enableStaleStores()`). No need to change the field name, though.
Maybe, we could also add this special rule for getters of boolean
values to the grammar. WDYT?

I have a final remark about the `StoreQueryParams`. I think it should
be immutable. That is more an implementation detail and we should
discuss it on the PR. Just wanted to mention it in advance. Probably
we should add also a rule for immutability to the grammar.

Best,
Bruno

On Wed, Jan 22, 2020 at 7:38 PM Navinder Brar
 wrote:
>
> +1 on changing to storeName() and includeStateStores(). We can add this to 
> grammar wiki as Matthias suggested.
>
> I have edited the KIP to remove "Deprecating" in favor of "Changing" and I 
> agree we can deprecate store(final String storeName, final 
> QueryableStoreType queryableStoreType).
>
> Thanks
> Navinder
>    On Thursday, 23 January, 2020, 07:28:38 am IST, Matthias J. Sax 
> wrote:
>
>  Thanks for the clarifications about the getters. I agree that it makes
> sense to move to the new pattern incrementally. Might be useful to
> create a Jira (or multiple?) to track this. It's an straight forward change.
>
> A nit about the KIP: it should only list the signature but not the full
> code of the implementation (ie, only package name and the class + method
> names; we can omit toString(), equals(), and hashCode(), too -- alo, no
> license header please ;))
>
>
> nit: `isIncludeStaleStores` -> `includeStaleStores` (the "is"-prefix
> reads clumsy and it's common in Kafka code base to omit the "get"-prefix
> for getters -- we should adopt this)
>
> @John: might be worth to include this in the Grammar wiki page?
>
> nit (similar as above):
>
>  - `getStoreName` -> `storeName`
>  - `getQueryableStoreType` -> `queryableStoreType`
>
>
> The KIP says
>
> > Deprecating the KafkaStreams#store(final String storeName, final 
> > QueryableStoreType queryableStoreType, final boolean includeStaleStores) 
> > in favour of the funtion mentioned below.
>
> We don't need to deprecate this method but we can remove it directly,
> because it was never release.
>
>
> What is the plan for
>
> > store(final String storeName, final QueryableStoreType 
> > queryableStoreType) {
>
> Given that the new `StoreQueryParams` allows to specify `storeName` and
> `queryableStoreType`, should we deprecate this method in favor of the
> new `store(StoreQueryParams)` overload?
>
>
> -Matthias
>
>
>
> On 1/22/20 10:06 AM, John Roesler wrote:
> > Thanks Navinder! I've also updated the motivation.
> >
> > Thanks,
> > -John
> >
> > On Wed, Jan 22, 2020, at 11:12, Navinder Brar wrote:
> >> I went through the grammar wiki page and since it is already agreed in
> >> principle I will change from constructor to below method and add the
> >> getters back.
> >> public static  StoreQueryParams fromNameAndType(
> >>  final String storeName,
> >>  final QueryableStoreType  queryableStoreType
> >> )
> >>
> >>
> >> Thanks,
> >> Navinder
> >>
> >>    On Wednesday, 22 January, 2020, 09:32:07 pm IST, John Roesler
> >>  wrote:
> >>
> >>  22) I'm specifically proposing to establish a new convention.
> >> The existing convention is fundamentally broken and has
> >> been costly both for users and maintainers. That is the purpose
> >> of the grammar I proposed. The plan is to implement  new APIs
> >> following the grammar and gradually to port old APIs to it.
> >>
> >> The grammar wiki page has plenty of justification, so I won't
> >> recapitulate it here.
> >>
> >> Thanks,
> >> -John
> >>
> >> On Wed, Jan 22, 2020, at 09:39, Navinder Brar wrote:
> >>> 10) Sure John, please go ahead.
> >>>
> >>> 21) I have no strong opinion on constructor vs static factory. If
> >>> everyone's okay with it, I can make 

Re: [DISCUSS] KIP-546: Add quota-specific APIs to the Admin Client, redux

[Brian Byrne]

Thanks Rajini,

1) Good catch, fixed.

2) You're right. We'd need to extend ClientQuotaCallback#quotaLimit or add
an alternate function. For the sake of an initial implementation, I'm going
to remove '--show-overridden', and a subsequent KIP will have to propose an
extents to ClientQuotaCallback to return more detailed information.

3) You're correct. I've removed the default.

4) The idea of the first iteration is be compatible with the existing API,
so no modification to start. The APIs should be kept consistent. If a user
wants to add custom functionality, say an entity type, they'll need to
update their ConfigEntityType any way, and the quotas APIs are meant to
handle that gracefully by accepting a String which can be propagated.

The catch is 'units'. Part of the reason for having a default unit was for
backwards compatibility, but maybe it's best to leave units out of the
initial design. This might lead to adding more configuration entries, but
it's also the most flexible option. Thoughts?

Thanks,
Brian


On Thu, Jan 23, 2020 at 4:57 AM Rajini Sivaram 
wrote:

> Hi Brian,
>
> Thanks for the KIP. Looks good, hope we finally get this in!
>
> A few comments:
>
> 1) All the Admin interface methods seem to be using method names starting
> with upper-case letter, should be lower-case to be follow conventions.
> 2) Effective quotas returns not only the actual effective quota, but also
> overridden values. I don't think this works with custom quota callbacks.
> 3) KIP says that all quotas are currently bytes-per-second and we will use
> RATE_BPS as the default. Request quotas are a percentage. So this doesn't
> quite work. We also need to consider how this works with custom quota
> callbacks. Can custom quota implementations define their own units?
> 4) We seem to be defining a new set of quota-related classes e.g. for quota
> types, but we haven't considered what we do with the existing API in
> org.apache.kafka.server.quota. Should we keep these consistent? Are we
> planning to deprecate some of those?
>
>
> Regards,
>
> Rajini
>
>
> On Wed, Jan 22, 2020 at 7:28 PM Brian Byrne  wrote:
>
> > Hi Jason,
> >
> > I agree on (1). It was Colin's original suggestion, too, but he had
> changed
> > his mind in preference for enums. Strings are the more generic way for
> now,
> > so hopefully Colin can share his thinking when he's back. The QuotaFilter
> > usage was an error, this has been corrected.
> >
> > For (2), the config-centric mode is what we have today in CommandConfig:
> > reading/altering the configuration as it's described. The
> > DescribeEffectiveClientQuotas would be resolving the various config
> entries
> > to see what actually applies to a particular entity. The examples are a
> > little trivial, but the resolution can become much more complicated as
> the
> > number of config entries grows.
> >
> > List/describe aren't perfect either. Perhaps describe/resolve are a
> better
> > pair, with DescribeEffectiveClientQuotas -> ResolveClientQuotas?
> >
> > I appreciate the feedback!
> >
> > Thanks,
> > Brian
> >
> >
> >
> > On Tue, Jan 21, 2020 at 12:09 PM Jason Gustafson 
> > wrote:
> >
> > > Hi Brian,
> > >
> > > Thanks for the proposal! I have a couple comments/questions:
> > >
> > > 1. I'm having a hard time understanding the point of
> `QuotaEntity.Type`.
> > It
> > > sounds like this might be just for convenience since the APIs are using
> > > string types. If so, I think it's a bit misleading to represent it as
> an
> > > enum. In particular, I cannot see how the UNKNOWN type would be used.
> The
> > > `PrincipalBuilder` plugin allows users to provide their own principal
> > type,
> > > so I think the API should be usable even for unknown entity types. Note
> > > also that we appear to be relying on this enum in `QuotaFilter` class.
> I
> > > think that should be changed to just a string?
> > >
> > > 2. It's a little annoying that we have two separate APIs to describe
> > client
> > > quotas. The names do not really make it clear which API someone should
> > use.
> > > It might just be a naming problem. In the command utility, it looks
> like
> > > you are using --list and --describe to distinguish the two. Perhaps the
> > > APIs can be named similarly: e.g. ListClientQuotas and
> > > DescribeClientQuotas. However, looking at the examples, it's still not
> > very
> > > clear to me why we need both options. Basically I'm finding the
> > > "config-centric" mode not very intuitive.
> > >
> > > Thanks,
> > > Jason
> > >
> > >
> > > On Fri, Jan 17, 2020 at 2:14 PM Brian Byrne 
> wrote:
> > >
> > > > Thanks Colin, I've updated the KIP with the relevant changes.
> > > >
> > > > On Fri, Jan 17, 2020 at 10:17 AM Colin McCabe 
> > > wrote:
> > > >
> > > > > I thought about this a little bit more, and maybe we can leave in
> the
> > > > > enums rather than going with strings.  But we need to have an
> > "UNKNOWN"
> > > > > value for all the enums, so that if a value that the client doesn't
> > > > > 

Re: [VOTE] KIP-559: Make the Kafka Protocol Friendlier with L7 Proxies

[Guozhang Wang]

Hello David,

Thanks for the KIP! I have read through the proposal and had one minor and
one meta comment. But overall it looks good to me!

1) The JoinGroupRequest format does not have any new fields proposed, so we
could either clarify that it is listed here but without modifications (only
version bumps) or just remove it from the wiki.

2) Could we consider adding a "protocol version" to allow brokers to select
the leader with the highest version? This thought is brought up in
https://cwiki.apache.org/confluence/display/KAFKA/KIP-429%3A+Kafka+Consumer+Incremental+Rebalance+Protocol#KIP-429:KafkaConsumerIncrementalRebalanceProtocol-LookingintotheFuture:AssignorVersion.
I'm fine with keeping this KIP's scope as is, just wondering if you feel
comfortable piggy-backing this change as well if we are going to bump up
the JoinGroupReq/Response anyways.


Guozhang


On Wed, Jan 22, 2020 at 9:10 AM Eno Thereska  wrote:

> This is awesome! +1 (non binding)
> Eno
>
> On Tue, Jan 21, 2020 at 10:00 PM Gwen Shapira  wrote:
> >
> > Thank you for the KIP. Awesomely cloud-native improvement :)
> >
> > +1 (binding)
> >
> >
> > On Tue, Jan 21, 2020, 9:35 AM David Jacot  wrote:
> >
> > > Hi all,
> > >
> > > I would like to start a vote on KIP-559: Make the Kafka Protocol
> Friendlier
> > > with L7 Proxies.
> > >
> > > The KIP is here:
> > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-559%3A+Make+the+Kafka+Protocol+Friendlier+with+L7+Proxies
> > >
> > > Thanks,
> > > David
> > >
>


-- 
-- Guozhang

Re: [VOTE] KIP-562: Allow fetching a key from a single partition rather than iterating over all the stores on an instance

[John Roesler]

Thanks, Navinder. It's just to give everyone a chance to object if they wanted 
to.
-John

On Thu, Jan 23, 2020, at 00:44, Navinder Brar wrote:
> Oh sorry, my bad. Will wait for another 12 hours. 
> 
> On Thursday, 23 January, 2020, 12:09:57 pm IST, Matthias J. Sax 
>  wrote:  
>  
>  Navinder,
> 
> a KIP vote must be open for 72h and cannot be closed earlier.
> 
> -Matthias
> 
> On 1/22/20 10:27 PM, Navinder Brar wrote:
> > Thanks, everyone for voting. KIP-562 has been accepted with binding votes 
> > from John, Matthias, and Guozhang. 
> > 
> >    On Thursday, 23 January, 2020, 09:40:07 am IST, Guozhang Wang 
> > wrote:  
> >  
> >  +1 (binding) from me as well.
> > 
> > On Wed, Jan 22, 2020 at 5:59 PM Matthias J. Sax 
> > wrote:
> > 
> >> I have a few minor comments (compare the DISCUSS thread), but overall
> >> the KIP looks good.
> >>
> >> +1 (binding)
> >>
> >>
> >> -Matthias
> >>
> >> On 1/22/20 10:09 AM, John Roesler wrote:
> >>> Thanks for updating the KIP, Navinder.
> >>>
> >>> I'm +1 (binding) on the current proposal
> >>>
> >>> Thanks,
> >>> -John
> >>>
> >>> On Tue, Jan 21, 2020, at 12:50, Navinder Brar wrote:
>  Thanks, Guozhang. I agree it makes total sense. I will make the
>  edits.~Navinder
> 
>      On Tuesday, 21 January, 2020, 11:00:32 pm IST, Guozhang Wang
>   wrote:
> 
>    Hello Navinder,
> 
>  Thanks for brining up this proposal. I made a quick pass on that and
>  overall I think I agree with your ideas. Just a few thoughts about the
>  public APIs:
> 
>  1) As we are adding a new overload to `KafkaStreams#store`, could we
> >> just
>  add the storeName and queryableStoreType as part of StoreQueryParam, and
>  leaving that the only parameter of the function?
> 
>  2) along with 1), for the static constructors, instead of iterating over
>  all possible combos I'd suggest we make constructors with only, say,
>  storeName, and then adding `withXXX()` setters to set other fields.
> >> This is
>  in case we want to add more param fields into the object, that we do not
>  need to exponentially adding and deprecating the static constructors.
> 
> 
>  Guozhang
> 
> 
>  On Mon, Jan 20, 2020 at 10:42 AM Navinder Brar
>   wrote:
> 
> > Hello all,
> >
> > I'd like to propose a vote to serve keys from a specific
> >> partition-store
> > instead of iterating over all the local stores of an instance to
> >> locate the
> > key, as which happens currently.
> > The full KIP is provided here:
> >
> >
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-562%3A+Allow+fetching+a+key+from+a+single+partition+rather+than+iterating+over+all+the+stores+on+an+instance
> >
> >
> > Thanks,
> > Navinder
> >
> 
> 
>  --
>  -- Guozhang
> 
> >>
> >>
> > 
>

Re: [VOTE] KIP-526: Reduce Producer Metadata Lookups for Large Number of Topics

[Brian Byrne]

Thanks Jason,

I'm in favor of the latter: metadata.max.idle.ms. I agree that describing
it as a "period" is inaccurate. With metadata.max.idle.ms, it also aligns
with metadata.max.age.ms for determining refresh period (which is an actual
period).

I've updated the docs.

Thanks,
Brian

On Wed, Jan 22, 2020 at 6:19 PM Jason Gustafson  wrote:

> Thanks for the proposal. Looks good overall. I wanted to suggest a possible
> name change. I was considering something like `idle.metadata.expiration.ms
> `
> or maybe `metadata.max.idle.ms`. Thoughts?
>
> -Jason
>
>
> On Tue, Jan 21, 2020 at 11:38 AM Guozhang Wang  wrote:
>
> > Got it.
> >
> > I was proposing that we do the "delayed async batch" but I think your
> > argument for complexity and pushing it out of the scope is convincing, so
> > instead I propose we do the synchronous mini batching still but obviously
> > it is already there :)  I'm +1 on the current proposal scope.
> >
> > Guozhang
> >
> > On Tue, Jan 21, 2020 at 10:16 AM Brian Byrne 
> wrote:
> >
> > > Hi Guozhang,
> > >
> > > Ah, sorry, I misunderstood. Actually, this is solved for us today. How
> > the
> > > producer works is that it maintains at most one inflight metadata fetch
> > > request at any time, where each request is tagged with the current
> > > (monotonically increasing) request version. This version is bumped
> > whenever
> > > a new topic is encountered, and metadata fetching will continue to
> > process
> > > while the latest metadata response's version is below the current
> > version.
> > >
> > > So if a metadata request is in flight, and a number of threads produce
> to
> > > new topics, they'll be added to the working set but the next metadata
> > > request won't take place until the outstanding one returns. So their
> > > updates will be batched together. As you suggest, we can have a simple
> > list
> > > that tracks unknown topics to isolate new vs. old topics.
> > >
> > > Thanks,
> > > Brian
> > >
> > >
> > >
> > > On Tue, Jan 21, 2020 at 10:04 AM Guozhang Wang 
> > wrote:
> > >
> > > > Hi Brian,
> > > >
> > > > I think I buy the complexity and extra end-to-end-latency argument :)
> > I'm
> > > > fine with delaying the asynchronous tech fetching to future works and
> > > keep
> > > > the current KIP's scope as-is for now. Under that case can we
> consider
> > > just
> > > > a minor implementation detail (since it is not affecting public APIs
> we
> > > > probably do not even need to list it, but just thinking loud here):
> > > >
> > > > In your proposal when we request for a topic of unknown metadata, we
> > are
> > > > going to directly set the topic name as that singleton in the
> request.
> > > I'm
> > > > wondering for the scenario that KAFKA-8904 described, if the
> > > producer#send
> > > > for thousands of new topics are triggered sequentially by a single
> > thread
> > > > or concurrent threads? If it's the latter, and we expect in such
> > > scenarios
> > > > we may have multiple topics being requests within a very short time,
> > then
> > > > we can probably do sth. like this internally in a synchronized
> manner:
> > > >
> > > > 1) put the topic name into a list, as "unknown topics", then
> > > > 2) exhaust the list, and put all topics from that list to the
> request;
> > if
> > > > the list is empty, it means it has been emptied by another thread so
> we
> > > > skip sending a new request and just wait for the returned metadata
> > > refresh.
> > > >
> > > > In most cases the list would just be a singleton with the one that
> > thread
> > > > has just enqueued, but under extreme scenarios it can help batching a
> > few
> > > > topic names probably (of course, I'm thinking about very extreme
> cases
> > > > here, assuming that's was what we've seen in 8904). Since these two
> > steps
> > > > are very light-weighted, doing that in a synchronized block would not
> > > hurt
> > > > the concurrency too much.
> > > >
> > > >
> > > > Guozhang
> > > >
> > > >
> > > > On Tue, Jan 21, 2020 at 9:39 AM Brian Byrne 
> > wrote:
> > > >
> > > > > Hi Guozhang,
> > > > >
> > > > > Your understanding of the rationale is accurate, and what you
> suggest
> > > is
> > > > > completely plausible, however I have a slightly different take on
> the
> > > > > situation.
> > > > >
> > > > > When the KIP was originally drafted, making KafkaProducer#send
> > > > asynchronous
> > > > > was one element of the changes (this is a little more general than
> > (a),
> > > > but
> > > > > has similar implications). As you're aware, doing so would allow
> new
> > > > topics
> > > > > to aggregate since the producer could continue to push new records,
> > > > whereas
> > > > > today the producer thread is blocked waiting for resolution.
> > > > >
> > > > > However, there were concerns about changing client behavior
> > > unexpectedly
> > > > in
> > > > > this manner, and the change isn't as trivial as one would hope. For
> > > > > example, we'd have to introduce an intermediate queue of records
> for

Re: Create KIP Permission

[Bill Bejeck]

Thomas, you're all set now.

-Bill

On Thu, Jan 23, 2020 at 10:17 AM Thomas Becker 
wrote:

> I'd like permission to create a KIP please. My confluence account is
> twbecker.
>
>
> 
>
> This email and any attachments may contain confidential and privileged
> material for the sole use of the intended recipient. Any review, copying,
> or distribution of this email (or any attachments) by others is prohibited.
> If you are not the intended recipient, please contact the sender
> immediately and permanently delete this email and any attachments. No
> employee or agent of TiVo is authorized to conclude any binding agreement
> on behalf of TiVo by email. Binding agreements with TiVo may only be made
> by a signed written agreement.
>

[jira] [Resolved] (KAFKA-7737) Consolidate InitProducerId API

[Jason Gustafson (Jira)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-7737?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jason Gustafson resolved KAFKA-7737.

Fix Version/s: 2.5.0
   Resolution: Fixed

> Consolidate InitProducerId API
> --
>
> Key: KAFKA-7737
> URL: https://issues.apache.org/jira/browse/KAFKA-7737
> Project: Kafka
>  Issue Type: Task
>  Components: producer 
>Reporter: Viktor Somogyi-Vass
>Assignee: Jason Gustafson
>Priority: Minor
>  Labels: exactly-once
> Fix For: 2.5.0
>
>
> We have two separate paths in the producer for the InitProducerId API: one 
> for the transactional producer and one for the idempotent producer. It would 
> be nice to find a way to consolidate these.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Create KIP Permission

[Thomas Becker]

I'd like permission to create a KIP please. My confluence account is twbecker.




This email and any attachments may contain confidential and privileged material 
for the sole use of the intended recipient. Any review, copying, or 
distribution of this email (or any attachments) by others is prohibited. If you 
are not the intended recipient, please contact the sender immediately and 
permanently delete this email and any attachments. No employee or agent of TiVo 
is authorized to conclude any binding agreement on behalf of TiVo by email. 
Binding agreements with TiVo may only be made by a signed written agreement.

Re: [VOTE] KIP-515: Hardened TLS Configs to ZooKeeper

[Ron Dagostino]

Hi everyone.  I discovered something minor while addressing the
AclAuthorizer config inheritance issue that I need to document.  The
minimum 3 days for voting is up and we could successfully conclude the
vote with 3 +1 binding votes and a +1 non-binding vote, but I'll leave
the vote open another day in case anyone needs to comment.  Here's the
information.

The "zookeeper.ssl.context.supplier.class" configuration doesn't
actually exist in ZooKeeper 3.5.6.  The ZooKeeper admin guide
documents it as being there, but it doesn't appear in the code.  This
means we can't support it in KIP-515, and I am removing it.  I checked
the latest ZooKeeper 3.6 SNAPSHOT, and it has been added.  So this
config could probably be added to Kafka via a new, small KIP if/when
we upgrade to ZooKeeper 3.6 (which looks to be in release-candidate
stage at the moment).


I've created https://issues.apache.org/jira/browse/KAFKA-9469 ("Add
zookeeper.ssl.context.supplier.class config if/when adopting ZooKeeper
3.6") for this issue.

Again, I will leave the vote open another day in case there needs to
be any comment or discussion about this.

Ron

On Wed, Jan 22, 2020 at 8:56 AM Ron Dagostino  wrote:
>
> Hi everyone.  While finishing the PR for this KIP I realized that the
> inheritance of TLS ZooKeeper configs that happens in the *authorizer*
> does not reflect he spirit of our discussion.  In particular, based on
> our inheritance discussion in the DISCUSS thread, the inheritance of
> authorizer configs needn't be as constrained as it is currently
> documented to be.  I am going to update the KIP as described below and
> will assume there are no objections if nobody comments as such on this
> VOTE thread.
>
> The KIP currently states that there is a limited inheritance for
> authorizer ZooKeeper TLS configs as follows: "Every config can be
> prefixed with "authorizer." for the case when
> kafka.security.authorizer.AclAuthorizer connects via TLS to a
> ZooKeeper quorum separate from the one that Kafka is using – this
> specific use case will be identified in the configuration by
> explicitly setting authorizer.zookeeper.ssl.client.enable=true."
>
> In other words, the authorizer inherits the broker's ZK TLS configs
> *unless* it explicitly indicates via
> authorizer.zookeeper.ssl.client.enable=true that it is going to use
> its own configs, in which case inheritance does not occur -- i.e.
> there is no overriding or merging going on where the broker's
> ZooKeeper TLS configs act as a base upon which any "authorizer."
> prefixed configs act as an overlay/override; instead, if you point to
> another ZooKeeper quorum and want to change anything related to TLS
> then you must restate everything.
>
> We had a discussion related to potentially inheriting a broker's
> *non-ZooKeeper* TLS configs.  Inheritance was desirable, and I came
> around to that way of thinking, but it turned out to be impossible to
> do given that the broker's non-ZooKeeper TLS configs are potentially
> stored in ZooKeeper.  Still, inheritance was desirable as a concept,
> so we should do it for the authorizer since the broker's *ZooKeeper*
> TLS configs are available in the config file.
>
> The KIP will now state that the broker's ZooKeeper TLS configs will
> act as a base config upon which any "authorizer." ZooKeeper TLS
> configs act as an overlay -- the configs are merged.  This is
> consistent with how the other "authorizer." configs for ZooKeeper work
> (connection/session timeouts and max inflight requests, for example).
> This means that the order of evaluation for any particular authorizer
> ZooKeeper TLS configuration will be:
>
> (1) system property
> (2) broker non-prefixed ZooKeeper TLS config
> (3) "authorizer." prefixed ZooKeeper TLS config
>
> Note that (1) + (2) simply yields the ZooKeeper TLS configs that the
> broker is using -- with (2) overlaying (1) -- so any "authorizer."
> prefixed ZooKeeper TLS configs are a true additional level of overlay
> (again, consistent with the behavior of the ZooKeeper configs for
> connection/session timeouts and max inflight requests).
>
> Ron
>
> On Mon, Jan 20, 2020 at 11:14 AM Manikumar  wrote:
> >
> > +1 (binding).
> >
> > Thanks for the KIP.
> >
> > On Mon, Jan 20, 2020 at 9:21 PM Rajini Sivaram 
> > wrote:
> >
> > > +1 (binding)
> > >
> > > Thanks for the KIP, Ron!
> > >
> > > Regards,
> > >
> > > Rajini
> > >
> > >
> > > On Mon, Jan 20, 2020 at 3:36 PM Gwen Shapira  wrote:
> > >
> > > > +1 (binding), this has been an on-going concern. Thank you for
> > > > addressing this, Ron.
> > > >
> > > > On Mon, Jan 20, 2020 at 5:22 AM Ron Dagostino  wrote:
> > > > >
> > > > > Hi everyone.  I would like to start a vote on KIP-515: Enable ZK
> > > > > client to use the new TLS supported authentication.
> > > > >
> > > > > The KIP is here:
> > > > >
> > > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-515%3A+Enable+ZK+client+to+use+the+new+TLS+supported+authentication
> > > > >
> > > > > The discussion 

[jira] [Created] (KAFKA-9469) Add zookeeper.ssl.context.supplier.class config if/when adopting ZooKeeper 3.6

[Ron Dagostino (Jira)]

Ron Dagostino created KAFKA-9469:


 Summary: Add zookeeper.ssl.context.supplier.class config if/when 
adopting ZooKeeper 3.6
 Key: KAFKA-9469
 URL: https://issues.apache.org/jira/browse/KAFKA-9469
 Project: Kafka
  Issue Type: New Feature
  Components: config
Reporter: Ron Dagostino
Assignee: Ron Dagostino


The "zookeeper.ssl.context.supplier.class" configuration doesn't actually exist 
in ZooKeeper 3.5.6.  The ZooKeeper admin guide documents it as being there, but 
it doesn't appear in the code.  This means we can't support it in KIP-515, and 
it has been removed from that KIP.
I checked the latest ZooKeeper 3.6 SNAPSHOT, and it has been added.  So this 
config could probably be added to Kafka via a new, small KIP if/when we upgrade 
to ZooKeeper 3.6 (which looks to be in release-candidate stage at the moment).



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Re: [DISCUSS] KIP-546: Add quota-specific APIs to the Admin Client, redux

[Rajini Sivaram]

Hi Brian,

Thanks for the KIP. Looks good, hope we finally get this in!

A few comments:

1) All the Admin interface methods seem to be using method names starting
with upper-case letter, should be lower-case to be follow conventions.
2) Effective quotas returns not only the actual effective quota, but also
overridden values. I don't think this works with custom quota callbacks.
3) KIP says that all quotas are currently bytes-per-second and we will use
RATE_BPS as the default. Request quotas are a percentage. So this doesn't
quite work. We also need to consider how this works with custom quota
callbacks. Can custom quota implementations define their own units?
4) We seem to be defining a new set of quota-related classes e.g. for quota
types, but we haven't considered what we do with the existing API in
org.apache.kafka.server.quota. Should we keep these consistent? Are we
planning to deprecate some of those?


Regards,

Rajini


On Wed, Jan 22, 2020 at 7:28 PM Brian Byrne  wrote:

> Hi Jason,
>
> I agree on (1). It was Colin's original suggestion, too, but he had changed
> his mind in preference for enums. Strings are the more generic way for now,
> so hopefully Colin can share his thinking when he's back. The QuotaFilter
> usage was an error, this has been corrected.
>
> For (2), the config-centric mode is what we have today in CommandConfig:
> reading/altering the configuration as it's described. The
> DescribeEffectiveClientQuotas would be resolving the various config entries
> to see what actually applies to a particular entity. The examples are a
> little trivial, but the resolution can become much more complicated as the
> number of config entries grows.
>
> List/describe aren't perfect either. Perhaps describe/resolve are a better
> pair, with DescribeEffectiveClientQuotas -> ResolveClientQuotas?
>
> I appreciate the feedback!
>
> Thanks,
> Brian
>
>
>
> On Tue, Jan 21, 2020 at 12:09 PM Jason Gustafson 
> wrote:
>
> > Hi Brian,
> >
> > Thanks for the proposal! I have a couple comments/questions:
> >
> > 1. I'm having a hard time understanding the point of `QuotaEntity.Type`.
> It
> > sounds like this might be just for convenience since the APIs are using
> > string types. If so, I think it's a bit misleading to represent it as an
> > enum. In particular, I cannot see how the UNKNOWN type would be used. The
> > `PrincipalBuilder` plugin allows users to provide their own principal
> type,
> > so I think the API should be usable even for unknown entity types. Note
> > also that we appear to be relying on this enum in `QuotaFilter` class. I
> > think that should be changed to just a string?
> >
> > 2. It's a little annoying that we have two separate APIs to describe
> client
> > quotas. The names do not really make it clear which API someone should
> use.
> > It might just be a naming problem. In the command utility, it looks like
> > you are using --list and --describe to distinguish the two. Perhaps the
> > APIs can be named similarly: e.g. ListClientQuotas and
> > DescribeClientQuotas. However, looking at the examples, it's still not
> very
> > clear to me why we need both options. Basically I'm finding the
> > "config-centric" mode not very intuitive.
> >
> > Thanks,
> > Jason
> >
> >
> > On Fri, Jan 17, 2020 at 2:14 PM Brian Byrne  wrote:
> >
> > > Thanks Colin, I've updated the KIP with the relevant changes.
> > >
> > > On Fri, Jan 17, 2020 at 10:17 AM Colin McCabe 
> > wrote:
> > >
> > > > I thought about this a little bit more, and maybe we can leave in the
> > > > enums rather than going with strings.  But we need to have an
> "UNKNOWN"
> > > > value for all the enums, so that if a value that the client doesn't
> > > > understand is returned, it can get translated to that.  This is what
> we
> > > did
> > > > with the ACLs API, and it worked out well.
> > > >
> > >
> > > Done. One thing I omitted here was that the API still accepts/returns
> > > Strings, since there may be plugins that specify their own types/units.
> > If
> > > we'd like to keep it this way, then the UNKNOWN may be unnecessary. Let
> > me
> > > know how you'd feel this is best resolved.
> > >
> > >
> > > > On balance, I think we should leave in "units."  It could be useful
> for
> > > > future-proofing.
> > > >
> > >
> > > Done. Also added a comment in the ClientQuotaCommand to default to
> > RATE_BPS
> > > if no unit is supplied to ease adoption.
> > >
> > >
> > > > Also, since there are other kinds of quotas not covered by this API,
> we
> > > > should rename DescribeQuotas -> DescribeClientQuotas, AlterQuotas ->
> > > > AlterClientQuotas, etc. etc.
> > > >
> > >
> > > Done. Updated command and script name, too.
> > >
> > >
> > > > Maybe QuotaFilter doesn't need a "rule" argument to its constructor
> > right
> > > > now.  We can just do literal matching for everything.  Like I said
> > > earlier,
> > > > I don't think people do a lot of prefixing of principal names.  When
> we
> > > > added the "prefix matching" stuff for 

[jira] [Resolved] (KAFKA-9464) Close the producer in completeShutdown

[Ted Yu (Jira)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-9464?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ted Yu resolved KAFKA-9464.
---
Resolution: Not A Problem

> Close the producer in completeShutdown
> --
>
> Key: KAFKA-9464
> URL: https://issues.apache.org/jira/browse/KAFKA-9464
> Project: Kafka
>  Issue Type: Bug
>Reporter: Ted Yu
>Assignee: Ted Yu
>Priority: Minor
>
> In StreamThread#completeShutdown, the producer (if not null) should be closed.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible

[Maulin Vasavada]

Hi all,

I have updated the KIP document with the current state of conclusions.
Please review it and see if we are ready to move to Voting!

Thanks
Maulin

On Wed, Jan 22, 2020 at 12:42 AM Maulin Vasavada 
wrote:

> Hi all,
>
> Finally I squeezed time and I've a suggested code changes shown at
> https://github.com/maulin-vasavada/kafka/pull/4/files for discussing this
> further. I'll update the KIP document soon. Meanwhile, can you please take
> a look and continue the discussion?
>
> One challenge is at:
> https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR89
>
> Thanks
> Maulin
>
>
> On Tue, Oct 22, 2019 at 11:13 PM Maulin Vasavada <
> maulin.vasav...@gmail.com> wrote:
>
>> bump! Clement/Rajini? Any responses based on the latest posts?
>>
>> On Wed, Oct 16, 2019 at 10:58 PM Maulin Vasavada <
>> maulin.vasav...@gmail.com> wrote:
>>
>>> bump!
>>>
>>> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada <
>>> maulin.vasav...@gmail.com> wrote:
>>>
 Hi Clement

 1) existing validation code will remain in SslFactory
 2) the createEngine() method in SslEngineBuilder will move to
 SslFactory and the client/server mode setting will go there (I documented
 this in the latest KIP update)

 In the current KIP I am proposing (as per the latest updates) to make
 SSLContext loading/configuration/creation pluggable. I am not suggesting we
 do/repeat anything that is already addressed by the existing Providers for
 SSLContext implementation. The createEngine() method (which will move to
 SslFactory) will call SslContextFactory.create() to get references to the
 SSLContext and then call SSLContext#createEngine(peer, host) and set
 client/server mode as it does today. I'll try to put that in a sequence
 diagram and update the KIP to make it clearer.

 So to your question about SslFactory returning SSLContext - I am saying
 register SslContextFactory interface to provide the SSLContext object
 instead and keep SslFactory more-or-less as it is today with some
 additional responsibility of createEngine() method.

 Thanks
 Maulin

 Thanks
 Maulin




 On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement <
 clement_pelle...@ibi.com> wrote:

> Can you clarify a few points for me?
>
> The two stumbling blocks we have are:
> 1) reuse of the validation code in the existing SslFactory
> 2) the client/server mode on the SSLEngine
>
> How do you deal with those issues in your new proposal?
>
> My use case is to register a custom SslFactory that returns an
> SSLContext previously created elsewhere in the application. Can your new
> proposal handle this use case?
>
> -Original Message-
> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com]
> Sent: Friday, October 11, 2019 2:13 AM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
> extensible
>
> Check this out-
>
> https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349
>
> This is exactly what I mean by using existing provider's SSLContext
> implementation and customizing it with our data points. The similar
> thing
> Kafka's SslEngineBuilder is doing right now.
>
> On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada <
> maulin.vasav...@gmail.com>
> wrote:
>
> > You meant JSSE not JCE right? We are not talking about cryptographic
> > providers we are talking about ssl providers hence JSSE.
> >
> > I do understand how JSSE Providers work and also the impact of
> multiple
> > JSSE providers with same algorithms in same JVM along with sequencing
> > challenges for the same.
> >
> > Like you said- we need to allow customizing the configuration for
> > SSLContext, so how many ways we have?
> >
> > Option-1: Write a custom JSSE Provider with our SSLContext
> >
> > Option-2: Use whichever SSLContext impl that you get from existing
> JSSE
> > Provider for SSLContext AND customize data for key material, trust
> material
> > AND secure random.
> >
> > Which one you prefer for this context?
> >
> > I feel we are making it complicated for no reason. It is very simple
> -
> > When we need to have SSL we need data points like - 1) Keys, 2)
> Trust certs
> > and 3) Secure Random which is feed to SSLContext and we are done. So
> we can
> > keep existing Kafka implementation as is by just making those data
> points
> > pluggable. Now SecureRandom is already pluggable via
> > 'ssl.secure.random.implementation' so that leaves us with keys and
> trusted
> > certs. For that purpose I raised KIP-486 BUT everybody feels we
> still need
> > higher 

Re: [DISCUSS] KIP-561: Regex Expressions Support for ConsumerGroupCommand

[Alexander Dunayevsky]

Hello guys,

Let's discuss KIP-561 Regex Support for ConsumerGroupCommand:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-561%3A+Regex+Support+for+ConsumerGroupCommand

Functionality already implemented and waiting to be reviewed.

Best Regards,
Alex Dunayevsky


On Thu, 16 Jan 2020, 14:25 Alex D,  wrote:

> Hello, guys,
>
> Please review Regex Expressions Support for ConsumerGroupCommand
> improvement proposal
>
>- *Previous Discussion 1*: Re: Multiple Consumer Group Management
>
>- *Previous Discussion 2*: Re: ConsumerGroupCommand tool improvement?
>
>
> *JIRA*: KAFKA-7817 Multiple Consumer Group Management with Regex
> 
>
> *PR*: #6700 
>