[jira] [Created] (KAFKA-13598) producer config didn't throw exception when retries and acks config changed

2022-01-18 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13598:
-

 Summary: producer config didn't throw exception when retries and 
acks config changed
 Key: KAFKA-13598
 URL: https://issues.apache.org/jira/browse/KAFKA-13598
 Project: Kafka
  Issue Type: Bug
  Components: clients, config
Affects Versions: 3.0.0, 3.1.0
Reporter: Luke Chen
Assignee: Luke Chen






--
This message was sent by Atlassian Jira
(v8.20.1#820001)


Re: [VOTE] KIP-811: Add config min.repartition.purge.interval.ms to Kafka Streams

2022-01-16 Thread Luke Chen
Hi Nick,

Thanks for the KIP!
+1 (non-binding)

Luke

On Sat, Jan 15, 2022 at 4:55 AM Matthias J. Sax  wrote:

> +1 (binding)
>
> On 1/14/22 06:32, John Roesler wrote:
> > Thanks for the KIP, Nick!
> >
> > +1 (binding)
> >
> > -John
> >
> > On Fri, Jan 14, 2022, at 07:40, Bruno Cadonna wrote:
> >> Hi Nick,
> >>
> >> Since the title of the KIP slightly changed after the vote was opened
> >> also the link to the KIP changed as a result. This is should be a
> >> working link:
> >>
> >> https://cwiki.apache.org/confluence/x/JY-kCw
> >>
> >> Anyways, Thanks for the KIP!
> >>
> >> I am +1 (binding)
> >>
> >> Best,
> >> Bruno
> >>
> >>
> >>
> >> On 12.01.22 16:34, Nick Telford wrote:
> >>> Hi everyone,
> >>>
> >>> I'd like to call a vote to adopt KIP-811: Add config
> >>> min.repartition.purge.interval.ms to Kafka Streams
> >>> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-811%3A+Add+config+min.repartition.purge.interval.ms+to+Kafka+Streams
> >
> >>> .
> >>>
> >>> Regards
> >>>
> >>> Nick Telford
> >>>
>


Re: log4j-2.x

2022-01-15 Thread Luke Chen
Hi Berthold,

Yes, we have a plan to upgrade from log4j-1.x to log4j 2.x, i.e. KIP-653

But sorry, we don't have timeline for this upgrading.
The PR is here: https://github.com/apache/kafka/pull/7898
Welcome to review/comment.

Thank you.
Luke

On Sat, Jan 15, 2022 at 5:14 PM Berthold Reinwald 
wrote:

> Hi Everyone,
>
>
>
> kafka_2.13-3.0.0 is using log4j-1.2.17. Is there a plan to upgrade from
> log4j-1.x to log4j 2.x, and what would be the time line?
>
> Regards,
>
> Berthold Reinwald
> reinw...@apache.org
>


Re: [DISCUSS] KIP-813: Replace KafkaConsumer with AdminClient in GetOffsetShell

2022-01-14 Thread Luke Chen
Hi Ziming,

Thanks for the KIP!
It's good to support fetch max timestamp in GetOffsetShell.

Some comments to the KIP:
1. It might be good and clear to list (some) current options in
`GetOffsetShell`, and the changes you want to make. You can refer to the
KIP-635 here

.
2. It's good to add some examples to show how we can get the max timestamp.
The examples in your PR is good start. You can add simple comment to
explain what the command is doing. Ex:

# fetch max timestamp
bin/kafka-run-class.sh kafka.tools.GetOffsetShell --bootstrap-server
localhost:9092 --topic topic1 --time -3
topic1:0:9979

and add 1 or 2 examples to show how to use the changed `--command-config`
config.

Thank you.
Luke

On Fri, Jan 14, 2022 at 9:42 PM deng ziming 
wrote:

> Hi everyone,
>
> I would like to start a discussion for KIP-813
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-813%3A+Replace+KafkaConsumer+with+AdminClient+in+GetOffsetShell
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-813:+Replace+KafkaConsumer+with+AdminClient+in+GetOffsetShell
> >
>
> The direct intention of this is to support max timestamp in
> GetOffsetShell, This seems like a simple change but there are some things
> to consider since it will change the --command-config parameter
>
> Let me know what you think.
>
>
> Thanks,
> Ziming Deng


Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2022-01-14 Thread Luke Chen
Hi Matthias,

Thanks for the comments.

1. The config name: `default.dsl.store` looks good to me. Updated!
2. the KIP still contains a view case of the originally proposed
config name `default.dsl.store.type` which should be updated.
--> Updated. Thanks.
3. About `TopologyConfig`: we should add all public methods of this class,
including constructors.
--> Updated

Thank you.
Luke


On Thu, Jan 13, 2022 at 11:48 AM Matthias J. Sax  wrote:

> Thanks for the KIP!
>
> I think it's a good step forward for the DSL and it makes sense to
> exclude the PAPI and custom stores for now.
>
> About the config name, it seems to be overly complicated. Guozhang's
> argument about "store type" that is used to refer to kv, windowed,
> session stores make sense. But having "type" and "impl" in the name
> sounds clumsy to me. What about a simplified name:
>
>default.dsl.store.impl
>
> or maybe even better
>
>default.dsl.store
>
> for the config?
>
> Btw: the KIP still contains a view case of the originally proposed
> config name `default.dsl.store.type` which should be updated.
>
>
> About `TopologyConfig`: we should add all public methods of this class,
> including constructors.
>
>
> -Matthias
>
> On 12/22/21 4:54 AM, Luke Chen wrote:
> > Hi Guozhang,
> >
> > Thanks for the comments.
> > And I agree it's better to rename it to `default.dsl.store.impl.type` for
> > differentiation.
> > I've updated the KIP.
> >
> > Thank you.
> > Luke
> >
> >
> > On Wed, Dec 22, 2021 at 3:12 AM Guozhang Wang 
> wrote:
> >
> >> Thanks Luke, I do not have any major comments on the wiki any more. BTW
> >> thanks for making the "public StreamsBuilder(final TopologyConfig
> >> topologyConfigs)" API public now, I think it will benefit a lot of
> future
> >> works!
> >>
> >> I think with the new API, we can deprecate the `build(props)` function
> in
> >> StreamsBuilder now, and will file a separate JIRA for it.
> >>
> >> Just a few nits:
> >>
> >> 1) There seems to be a typo at the end: "ROCK_DB"
> >> 2) Sometimes people refer to "store type" as kv-store, window-store etc;
> >> maybe we can differentiate them a bit by calling the new API names
> >> `StoreImplType`,
> >> `default.dsl.store.impl.type` and `The default store implementation type
> >> used by DSL operators`.
> >>
> >> On Thu, Dec 16, 2021 at 2:29 AM Luke Chen  wrote:
> >>
> >>> Hi Guozhang,
> >>>
> >>> I've updated the KIP to use `enum`, instead of store implementation,
> and
> >>> some content accordingly.
> >>> Please let me know if there's other comments.
> >>>
> >>>
> >>> Thank you.
> >>> Luke
> >>>
> >>> On Sun, Dec 12, 2021 at 3:55 PM Luke Chen  wrote:
> >>>
> >>>> Hi Guozhang,
> >>>>
> >>>> Thanks for your comments.
> >>>> I agree that in the KIP, there's a trade-off regarding the API
> >>> complexity.
> >>>> With the store impl, we can support default custom stores, but
> >> introduce
> >>>> more complexity for users, while with the enum types, users can
> >> configure
> >>>> default built-in store types easily, but it can't work for custom
> >> stores.
> >>>>
> >>>> For me, I'm OK to narrow down the scope and introduce the default
> >>> built-in
> >>>> enum store types first.
> >>>> And if there's further request, we can consider a better way to
> support
> >>>> default store impl.
> >>>>
> >>>> I'll update the KIP next week, unless there are other opinions from
> >> other
> >>>> members.
> >>>>
> >>>> Thank you.
> >>>> Luke
> >>>>
> >>>> On Fri, Dec 10, 2021 at 6:33 AM Guozhang Wang 
> >>> wrote:
> >>>>
> >>>>> Thanks Luke for the updated KIP.
> >>>>>
> >>>>> One major change the new proposal has it to change the original enum
> >>> store
> >>>>> type with a new interface. Where in the enum approach our internal
> >>>>> implementations would be something like:
> >>>>>
> >>>>> "
> >>>>> Stores#keyValueBytesStoreSupplier(storeImplTypes, storeName, ...)
> >>>>

[jira] [Created] (KAFKA-13589) fix flaky `PlaintextAdminIntegrationTest.testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords` test

2022-01-10 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13589:
-

 Summary: fix flaky 
`PlaintextAdminIntegrationTest.testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords`
 test
 Key: KAFKA-13589
 URL: https://issues.apache.org/jira/browse/KAFKA-13589
 Project: Kafka
  Issue Type: Test
Reporter: Luke Chen
Assignee: Luke Chen


org.opentest4j.AssertionFailedError: Expected follower to catch up to log end 
offset 200 at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:39) 
at org.junit.jupiter.api.Assertions.fail(Assertions.java:117) at 
kafka.api.PlaintextAdminIntegrationTest.waitForFollowerLog$1(PlaintextAdminIntegrationTest.scala:730)
 at 
kafka.api.PlaintextAdminIntegrationTest.testReplicaCanFetchFromLogStartOffsetAfterDeleteRecords(PlaintextAdminIntegrationTest.scala:760)



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


Re: [VOTE] 3.1.0 RC0

2022-01-07 Thread Luke Chen
Hi David,

I've done:
1. Verified checksums and signatures
2. Ran quick start using scala 2.13
3. Browse the java doc

All looks good.
+1 (non-binding)

Thanks for running the release.

Luke


On Fri, Jan 7, 2022 at 11:38 PM Israel Ekpo  wrote:

> So far we have 1 binding +1 vote and 4 non-binding +1 votes.
>
> Could we get more participation from community members with binding votes
> so close it out by the deadline today?
>
> We have approximately 13 hours left for voting
>
> Thanks
>
> Israel Ekpo
> Lead Instructor, IzzyAcademy.com
> https://www.youtube.com/c/izzyacademy
> https://izzyacademy.com/
>
>
> On Mon, Jan 3, 2022 at 6:53 PM Jakub Scholz  wrote:
>
> > +1 (non-binding). I used the staged Scala 2.13 binaries and the staging
> > Maven repository to run my tests. All seems to work fine.
> >
> > Thanks for running the release David!
> >
> > Jakub
> >
> > On Thu, Dec 23, 2021 at 11:17 PM David Jacot  wrote:
> >
> > > Hello Kafka users, developers and client-developers,
> > >
> > > This is the first candidate for release of Apache Kafka 3.1.0.
> > >
> > > * Apache Kafka supports Java 17
> > > * The FetchRequest supports Topic IDs (KIP-516)
> > > * Extend SASL/OAUTHBEARER with support for OIDC (KIP-768)
> > > * Add broker count metrics (KIP-748)
> > > * Differentiate consistently metric latency measured in millis and
> > > nanos (KIP-773)
> > > * The eager rebalance protocol is deprecated (KAFKA-13439)
> > > * Add TaskId field to StreamsException (KIP-783)
> > > * Custom partitioners in foreign-key joins (KIP-775)
> > > * Fetch/findSessions queries with open endpoints for
> > > SessionStore/WindowStore (KIP-766)
> > > * Range queries with open endpoints (KIP-763)
> > > * Add total blocked time metric to Streams (KIP-761)
> > > * Add additional configuration to control MirrorMaker2 internal topics
> > > naming convention (KIP-690)
> > >
> > > Release notes for the 3.1.0 release:
> > > https://home.apache.org/~dajac/kafka-3.1.0-rc0/RELEASE_NOTES.html
> > >
> > > *** Please download, test and vote by Friday, January 7, 9am PT
> > >
> > > Kafka's KEYS file containing PGP keys we use to sign the release:
> > > https://kafka.apache.org/KEYS
> > >
> > > * Release artifacts to be voted upon (source and binary):
> > > https://home.apache.org/~dajac/kafka-3.1.0-rc0/
> > >
> > > * Maven artifacts to be voted upon:
> > > https://repository.apache.org/content/groups/staging/org/apache/kafka/
> > >
> > > * Javadoc:
> > > https://home.apache.org/~dajac/kafka-3.1.0-rc0/javadoc/
> > >
> > > * Tag to be voted upon (off 3.1 branch) is the 3.1.0 tag:
> > > https://github.com/apache/kafka/releases/tag/3.1.0-rc0
> > >
> > > * Documentation:
> > > https://kafka.apache.org/31/documentation.html
> > >
> > > * Protocol:
> > > https://kafka.apache.org/31/protocol.html
> > >
> > > * Successful Jenkins builds for the 3.1 branch:
> > > Unit/integration tests:
> > > https://ci-builds.apache.org/job/Kafka/job/kafka/job/3.1/42/
> > > System tests:
> > > https://jenkins.confluent.io/job/system-test-kafka/job/3.1/44/
> > >
> > > /**
> > >
> > > Thanks,
> > > David
> > >
> >
>


Re: [VOTE] KIP-810: Allow producing records with null values in Kafka Console Producer

2022-01-04 Thread Luke Chen
Hi Mickael,

Thanks for the KIP!

+1 (non-binding) from me.

Thank you.
Luke

On Tue, Jan 4, 2022 at 6:24 PM Mickael Maison 
wrote:

> Hi,
>
> I'd like to start a vote on KIP-810 that adds the option to produce
> records with a null value
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-810%3A+Allow+producing+records+with+null+values+in+Kafka+Console+Producer
>
> Let me know if you have any feedback.
>
> Thanks,
> Mickael
>


[jira] [Created] (KAFKA-13567) adminClient exponential backoff implementation

2021-12-24 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13567:
-

 Summary: adminClient exponential backoff implementation
 Key: KAFKA-13567
 URL: https://issues.apache.org/jira/browse/KAFKA-13567
 Project: Kafka
  Issue Type: Sub-task
Reporter: Luke Chen






--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Created] (KAFKA-13566) producer exponential backoff implementation

2021-12-24 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13566:
-

 Summary: producer exponential backoff implementation
 Key: KAFKA-13566
 URL: https://issues.apache.org/jira/browse/KAFKA-13566
 Project: Kafka
  Issue Type: Sub-task
Reporter: Luke Chen






--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Created] (KAFKA-13565) consumer exponential backoff implementation

2021-12-24 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13565:
-

 Summary: consumer exponential backoff implementation
 Key: KAFKA-13565
 URL: https://issues.apache.org/jira/browse/KAFKA-13565
 Project: Kafka
  Issue Type: Sub-task
Reporter: Luke Chen






--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Created] (KAFKA-13563) Consumer failure after rolling Broker upgrade

2021-12-22 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13563:
-

 Summary: Consumer failure after rolling Broker upgrade
 Key: KAFKA-13563
 URL: https://issues.apache.org/jira/browse/KAFKA-13563
 Project: Kafka
  Issue Type: Bug
  Components: clients
Reporter: Luke Chen
Assignee: Luke Chen


This failure occurred again during this month's rolling OS security updates to 
the Brokers (no change to Broker version).  I have also been able to reproduce 
it locally with the following process:
 
1. Start a 3 Broker cluster with a Topic having Replicas=3.
2. Start a Client with Producer and Consumer communicating over the Topic.
3. Stop the Broker that is acting as the Group Coordinator.
4. Observe successful Rediscovery of new Group Coordinator.
5. Restart the stopped Broker.
6. Stop the Broker that became the new Group Coordinator at step 4.
7. Observe "Rediscovery will be attempted" message but no "Discovered group 
coordinator" message.
 
In short, Group Coordinator Rediscovery only works for the first Broker 
failover not any subsequent failover.
 
I conducted tests using 2.7.1 servers.  The issue occurs with 2.7.1 and 2.7.2 
Clients.  The issue does not occur with 2.5.1 and 2.7.0 Clients.  This make me 
suspect that https://issues.apache.org/jira/browse/KAFKA-10793 introduced this 
issue.
 
 

 



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-22 Thread Luke Chen
Hi Guozhang,

Thanks for the comments.
And I agree it's better to rename it to `default.dsl.store.impl.type` for
differentiation.
I've updated the KIP.

Thank you.
Luke


On Wed, Dec 22, 2021 at 3:12 AM Guozhang Wang  wrote:

> Thanks Luke, I do not have any major comments on the wiki any more. BTW
> thanks for making the "public StreamsBuilder(final TopologyConfig
> topologyConfigs)" API public now, I think it will benefit a lot of future
> works!
>
> I think with the new API, we can deprecate the `build(props)` function in
> StreamsBuilder now, and will file a separate JIRA for it.
>
> Just a few nits:
>
> 1) There seems to be a typo at the end: "ROCK_DB"
> 2) Sometimes people refer to "store type" as kv-store, window-store etc;
> maybe we can differentiate them a bit by calling the new API names
> `StoreImplType`,
> `default.dsl.store.impl.type` and `The default store implementation type
> used by DSL operators`.
>
> On Thu, Dec 16, 2021 at 2:29 AM Luke Chen  wrote:
>
> > Hi Guozhang,
> >
> > I've updated the KIP to use `enum`, instead of store implementation, and
> > some content accordingly.
> > Please let me know if there's other comments.
> >
> >
> > Thank you.
> > Luke
> >
> > On Sun, Dec 12, 2021 at 3:55 PM Luke Chen  wrote:
> >
> > > Hi Guozhang,
> > >
> > > Thanks for your comments.
> > > I agree that in the KIP, there's a trade-off regarding the API
> > complexity.
> > > With the store impl, we can support default custom stores, but
> introduce
> > > more complexity for users, while with the enum types, users can
> configure
> > > default built-in store types easily, but it can't work for custom
> stores.
> > >
> > > For me, I'm OK to narrow down the scope and introduce the default
> > built-in
> > > enum store types first.
> > > And if there's further request, we can consider a better way to support
> > > default store impl.
> > >
> > > I'll update the KIP next week, unless there are other opinions from
> other
> > > members.
> > >
> > > Thank you.
> > > Luke
> > >
> > > On Fri, Dec 10, 2021 at 6:33 AM Guozhang Wang 
> > wrote:
> > >
> > >> Thanks Luke for the updated KIP.
> > >>
> > >> One major change the new proposal has it to change the original enum
> > store
> > >> type with a new interface. Where in the enum approach our internal
> > >> implementations would be something like:
> > >>
> > >> "
> > >> Stores#keyValueBytesStoreSupplier(storeImplTypes, storeName, ...)
> > >> Stores#windowBytesStoreSupplier(storeImplTypes, storeName, ...)
> > >> Stores#sessionBytesStoreSupplier(storeImplTypes, storeName, ...)
> > >> "
> > >>
> > >> And inside the impl classes like here we would could directly do:
> > >>
> > >> "
> > >> if ((supplier = materialized.storeSupplier) == null) {
> > >> supplier =
> > >> Stores.windowBytesStoreSupplier(materialized.storeImplType())
> > >> }
> > >> "
> > >>
> > >> While I understand the benefit of having an interface such that user
> > >> customized stores could be used as the default store types as well,
> > >> there's
> > >> a trade-off I feel regarding the API complexity. Since with this
> > approach,
> > >> our API complexity granularity is in the order of "number of impl
> > types" *
> > >> "number of store types". This means that whenever we add new store
> types
> > >> in
> > >> the future, this API needs to be augmented and customized impl needs
> to
> > be
> > >> updated to support the new store types, in addition, not all custom
> impl
> > >> types may support all store types, but with this interface they are
> > forced
> > >> to either support all or explicitly throw un-supported exceptions.
> > >>
> > >> The way I see a default impl type is that, they would be safe to use
> for
> > >> any store types, and since store types are evolved by the library
> > itself,
> > >> the default impls would better be controlled by the library as well.
> > >> Custom
> > >> impl classes can choose to replace some of the stores explicitly, but
> > may
> > >> not be a best fit as the default impl classes --- if there are in the
> > >&g

Re: [DISCUSS] KIP-811 Add separate delete.interval.ms to Kafka Streams

2021-12-21 Thread Luke Chen
Thanks, Bruno.

I agree my point 4 is more like PR discussion, not KIP discussion.
@Nick , please update my point 4 in PR directly.

Thank you.
Luke




On Tue, Dec 21, 2021 at 7:24 PM Bruno Cadonna  wrote:

> Hi Nick,
>
> Thank you for the KIP!
>
> I agree on points 1, 2, and 3. I am not sure about point 4. I agree that
> we should update the docs for commit.interval.ms but I am not sure if
> this needs to mentioned in the KIP. That seems to me more a PR
> discussion. Also on point 2, I agree that we need to add a doc string
> but the content should be exemplary not binding. What I want to say is
> that, we do not need a KIP to change docs.
>
> Here my points:
>
> 5. Could you specify in the motivation that the KIP is about deleting
> records from repartition topics? Maybe with a short description when why
> and when records are deleted from the repartition topics. For us it
> might be clear, but IMO we should try to write KIPs so that someone that
> is relatively new to Kafka Streams can understand the KIP without
> needing to know too much background.
>
> 6. Does the config need to be validated? For example, does
> delete.interval.ms need to be greater than or equal to commit.interval.ms?
>
> 7. Should the default value for non-EOS be 30s or the same value as
> commit.interval.ms? I am just thinking about the case where a user
> explicitly changes commit.interval.ms but not delete.interval.ms (or
> whatever name you come up for it). Once delete.interval.ms is set
> explicitly it is decoupled from commit.interval.ms. Similar could be
> done for the EOS case.
> Alternatively, we could also define delete.interval.ms to take a
> integral number without a unit that specifies after how many commit
> intervals the records in repartition topics should be deleted. This
> would make sense since delete.interval.ms is tightly bound to
> commit.interval.ms. Additionally, it would make the semantics of the
> config simpler. The name of the config should definitely change if we go
> down this way.
>
> Best,
> Bruno
>
>
>
> On 21.12.21 11:14, Luke Chen wrote:
> > Hi Nick,
> >
> > Thanks for the KIP!
> >
> > In addition to Sophie's comments, I have one more to this KIP:
> > 3. I think you should mention the behavior change *explicitly* in
> > "Compatibility" section. I know you already mentioned it in KIP, in the
> > benefit way. But I think in this section, we should clearly point out
> which
> > behavior will be change after this KIP. That is, you should put it clear
> > that the delete record interval will change from 100ms to 30s with EOS
> > enabled. And it should also be mentioned in doc/upgrade.html doc.
> > 4. Since this new config has some relationship with commit.interval.ms,
> I
> > think we should also update the doc description for `commit.interval.ms
> `,
> > to let user know there's another config to control delete interval and
> > should be greater than commit.interval.ms. Something like that. WDYT?
> (You
> > should put this change in the KIP as Sophie mentioned)
> >
> > Thank you.
> > Luke
> >
> > On Tue, Dec 21, 2021 at 9:27 AM Sophie Blee-Goldman
> >  wrote:
> >
> >> Hey Nick,
> >>
> >> I think you forgot to link to the KIP document, but I take it this is
> >> it: KIP-811:
> >> Add separate delete.interval.ms to Kafka Streams
> >> <https://cwiki.apache.org/confluence/x/JY-kCw>
> >>
> >> The overall proposal sounds good to me, just a few minor things:
> >>
> >> 1. Please specify everything needed to define this config
> explicitly, ie
> >> all the arguments that will be passed in to the
> >> StreamsConfig's ConfigDef: in addition to the default value, we
> need the
> >> config type (presumably a Long), the doc
> >> string, and the importance (probably "low", similar to
> >> commit.interval.ms
> >> )
> >> 2. Might be worth considering a slightly more descriptive name for
> this
> >> config. Most users probably don't think about,
> >> or may not even be aware of, the deletion of consumed records by
> Kafka
> >> Streams, so calling it something along
> >> the lines of "repartition.records.delete.interval.ms" or "
> >> consumed.records.deletion.interval.ms" or so on will help
> >> make it clear what the config refers to and whether or not they
> need to
> >> care. Maybe you can come up with better
> >> and/or shorter names, just wanted to suggest some example names
> that I
> >> think sufficiently get the point across
> >>
> >> Other than that I'm +1 -- thanks for the KIP!
> >>
> >> Sophie
> >>
> >>
> >>
> >> On Mon, Dec 20, 2021 at 9:15 AM Nick Telford 
> >> wrote:
> >>
> >>> This is a KIP for a proposed solution to KAFKA-13549
> >>> <https://issues.apache.org/jira/browse/KAFKA-13549>. The solution is
> >> very
> >>> simple, so the KIP is pretty short.
> >>>
> >>> The suggested changes are implemented by this PR
> >>> <https://github.com/apache/kafka/pull/11610>.
> >>> --
> >>> Nick Telford
> >>>
> >>
> >
>


Re: [DISCUSS] KIP-811 Add separate delete.interval.ms to Kafka Streams

2021-12-21 Thread Luke Chen
Hi Nick,

Thanks for the KIP!

In addition to Sophie's comments, I have one more to this KIP:
3. I think you should mention the behavior change *explicitly* in
"Compatibility" section. I know you already mentioned it in KIP, in the
benefit way. But I think in this section, we should clearly point out which
behavior will be change after this KIP. That is, you should put it clear
that the delete record interval will change from 100ms to 30s with EOS
enabled. And it should also be mentioned in doc/upgrade.html doc.
4. Since this new config has some relationship with commit.interval.ms, I
think we should also update the doc description for `commit.interval.ms`,
to let user know there's another config to control delete interval and
should be greater than commit.interval.ms. Something like that. WDYT? (You
should put this change in the KIP as Sophie mentioned)

Thank you.
Luke

On Tue, Dec 21, 2021 at 9:27 AM Sophie Blee-Goldman
 wrote:

> Hey Nick,
>
> I think you forgot to link to the KIP document, but I take it this is
> it: KIP-811:
> Add separate delete.interval.ms to Kafka Streams
> 
>
> The overall proposal sounds good to me, just a few minor things:
>
>1. Please specify everything needed to define this config explicitly, ie
>all the arguments that will be passed in to the
>StreamsConfig's ConfigDef: in addition to the default value, we need the
>config type (presumably a Long), the doc
>string, and the importance (probably "low", similar to
> commit.interval.ms
>)
>2. Might be worth considering a slightly more descriptive name for this
>config. Most users probably don't think about,
>or may not even be aware of, the deletion of consumed records by Kafka
>Streams, so calling it something along
>the lines of "repartition.records.delete.interval.ms" or "
>consumed.records.deletion.interval.ms" or so on will help
>make it clear what the config refers to and whether or not they need to
>care. Maybe you can come up with better
>and/or shorter names, just wanted to suggest some example names that I
>think sufficiently get the point across
>
> Other than that I'm +1 -- thanks for the KIP!
>
> Sophie
>
>
>
> On Mon, Dec 20, 2021 at 9:15 AM Nick Telford 
> wrote:
>
> > This is a KIP for a proposed solution to KAFKA-13549
> > . The solution is
> very
> > simple, so the KIP is pretty short.
> >
> > The suggested changes are implemented by this PR
> > .
> > --
> > Nick Telford
> >
>


Re: [VOTE] KIP-792: Add "generation" field into consumer protocol

2021-12-20 Thread Luke Chen
Hi all,

Bump this thread to see if there are other comments to this KIP.

Thank you.
Luke

On Fri, Dec 17, 2021 at 1:27 AM Colin McCabe  wrote:

> Thanks for the explanation, Luke. That makes sense.
>
> best,
> Colin
>
> On Thu, Dec 9, 2021, at 13:31, Guozhang Wang wrote:
> > Thanks Luke, in that case I'm +1 on this KIP.
> >
> > On Thu, Dec 9, 2021 at 1:46 AM Luke Chen  wrote:
> >
> >> Hi Guozhang,
> >>
> >> Thanks for your comment.
> >>
> >> > we need to make sure the old-versioned leader would be able to ignore
> the
> >> new
> >> field during an upgrade e.g. without crashing.
> >>
> >> Yes, I understand. I'll be careful to make sure it won't crash the old
> >> versioned leader.
> >> But basically, it won't, because we appended the new field into the
> last of
> >> the ConsumerProtocolSubscription, which means, when read/deserialize the
> >> Subscription metadata, the old versioned leader will just read the head
> >> part of the data.
> >>
> >> Thanks for the reminder!
> >>
> >> Luke
> >>
> >> On Thu, Dec 9, 2021 at 4:00 AM Guozhang Wang 
> wrote:
> >>
> >> > Hi Luke,
> >> >
> >> > Thanks for the KIP.
> >> >
> >> > One thing I'd like to double check is that, since the
> >> > ConsumerProtocolSubscription is not auto generated from the json
> file, we
> >> > need to make sure the old-versioned leader would be able to ignore the
> >> new
> >> > field during an upgrade e.g. without crashing. Other than that, the
> KIP
> >> > lgtm.
> >> >
> >> > Guozhang
> >> >
> >> > On Tue, Dec 7, 2021 at 6:16 PM Luke Chen  wrote:
> >> >
> >> > > Hi Colin,
> >> > >
> >> > > I'm not quite sure if I understand your thoughts correctly.
> >> > > If I was wrong, please let me know.
> >> > >
> >> > > Also, I'm not quite sure how I could lock this feature to a new IBP
> >> > > version.
> >> > > I saw "KIP-584: Versioning scheme for features" is still under
> >> > development.
> >> > > Not sure if I need to lock the IBP version, how should I do?
> >> > >
> >> > > Thank you.
> >> > > Luke
> >> > >
> >> > > On Tue, Dec 7, 2021 at 9:41 PM Luke Chen  wrote:
> >> > >
> >> > > > Hi Colin,
> >> > > >
> >> > > > Thanks for your comments. I've updated the KIP to mention about
> the
> >> KIP
> >> > > > won't affect current broker side behavior.
> >> > > >
> >> > > > > One scenario that we need to consider is what happens during a
> >> > rolling
> >> > > > upgrade. If the coordinator moves back and forth between brokers
> with
> >> > > > different IBPs, it seems that the same epoch numbers could be
> reused
> >> > for
> >> > > a
> >> > > > group, if things are done in the obvious manner (old IBP = don't
> read
> >> > or
> >> > > > write epoch, new IBP = do)
> >> > > >
> >> > > > I think this KIP doesn't care about the group epoch number at all.
> >> The
> >> > > > subscription metadata is passed from each member to group
> >> coordinator,
> >> > > and
> >> > > > then the group coordinator pass all of them back to the consumer
> >> lead.
> >> > So
> >> > > > even if the epoch number is reused in a group, it should be fine.
> On
> >> > the
> >> > > > other hand, the group coordinator will have no idea if the join
> group
> >> > > > request sent from consumer containing the new subscription
> >> "generation"
> >> > > > field or not, because group coordinator won't deserialize the
> >> metadata.
> >> > > >
> >> > > > I've added also added them into the KIP.
> >> > > >
> >> > > > Thank you.
> >> > > > Luke
> >> > > >
> >> > > > On Mon, Dec 6, 2021 at 10:39 AM Colin McCabe 
> >> > wrote:
> >> > > >
> >> > > >> Hi Luke,
> >> > > >>
> >> > > >> Thanks for the explanation.
> >&

Re: [ANNOUNCE] New Kafka PMC member: David Jacot

2021-12-17 Thread Luke Chen
Congrats, David!
Well deserved.

Luke

deng ziming  於 2021年12月18日 週六 上午7:47 寫道:

> Congrats David!
>
> --
> Ziming Deng
>
> > On Dec 18, 2021, at 7:08 AM, Gwen Shapira  wrote:
> >
> > Hi everyone,
> >
> > David Jacot has been an Apache Kafka committer since Oct 2020 and has
> been contributing to the community consistently this entire time -
> especially notable the fact that he reviewed around 150 PRs in the last
> year. It is my pleasure to announce that David agreed to join the Kafka PMC.
> >
> > Congratulations, David!
> >
> > Gwen Shapira, on behalf of Apache Kafka PMC
>
>


Re: [DISCUSS] KIP-810: Allow producing records with null values in Kafka Console Producer

2021-12-16 Thread Luke Chen
Hi Mickael,

Thanks for the KIP!
This will be a helpful feature for debugging, for sure!

I have one question:
Will we have some safe net for the collision of `key.separator` and the new
introduced `null.marker`.
That is, what if user set the same or overlapped  `key.separator` and
`null.marker`, how would we handle it?
Ex: key.separator="-", null.marker="--".
Maybe it's corner case, but I think it'd be better we handle it gracefully.

Thank you.
Luke



On Wed, Dec 15, 2021 at 11:08 PM Chris Egerton 
wrote:

> Hi Mickael,
>
> Thanks for the KIP. Given how important tombstone records are it's hard to
> believe that the console producer doesn't already support them!
>
> I wanted to clarify the intended behavior and how it will play with the
> parse.key and the newly-introduced (as of KIP-798 [1]) parse.headers
> properties. Is the intention that the null.marker should match the entire
> line read by the console producer, or that it can match individual portions
> of a line that correspond to the record's key, value, header key, or header
> value? I imagine so but think it may be worth calling out (and possibly
> illustrating with an example or two) in the KIP.
>
> [1] -
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-798%3A+Add+possibility+to+write+kafka+headers+in+Kafka+Console+Producer
>
> Cheers,
>
> Chris
>
> On Wed, Dec 15, 2021 at 6:08 AM Mickael Maison 
> wrote:
>
> > Hi all,
> >
> > I opened a KIP to add the option to produce records with a null value
> > using the Console Producer:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-810%3A+Allow+producing+records+with+null+values+in+Kafka+Console+Producer
> >
> > Let me know if you have any feedback.
> >
> > Thanks,
> > Mickael
> >
>


Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-16 Thread Luke Chen
Hi Guozhang,

I've updated the KIP to use `enum`, instead of store implementation, and
some content accordingly.
Please let me know if there's other comments.


Thank you.
Luke

On Sun, Dec 12, 2021 at 3:55 PM Luke Chen  wrote:

> Hi Guozhang,
>
> Thanks for your comments.
> I agree that in the KIP, there's a trade-off regarding the API complexity.
> With the store impl, we can support default custom stores, but introduce
> more complexity for users, while with the enum types, users can configure
> default built-in store types easily, but it can't work for custom stores.
>
> For me, I'm OK to narrow down the scope and introduce the default built-in
> enum store types first.
> And if there's further request, we can consider a better way to support
> default store impl.
>
> I'll update the KIP next week, unless there are other opinions from other
> members.
>
> Thank you.
> Luke
>
> On Fri, Dec 10, 2021 at 6:33 AM Guozhang Wang  wrote:
>
>> Thanks Luke for the updated KIP.
>>
>> One major change the new proposal has it to change the original enum store
>> type with a new interface. Where in the enum approach our internal
>> implementations would be something like:
>>
>> "
>> Stores#keyValueBytesStoreSupplier(storeImplTypes, storeName, ...)
>> Stores#windowBytesStoreSupplier(storeImplTypes, storeName, ...)
>> Stores#sessionBytesStoreSupplier(storeImplTypes, storeName, ...)
>> "
>>
>> And inside the impl classes like here we would could directly do:
>>
>> "
>> if ((supplier = materialized.storeSupplier) == null) {
>> supplier =
>> Stores.windowBytesStoreSupplier(materialized.storeImplType())
>> }
>> "
>>
>> While I understand the benefit of having an interface such that user
>> customized stores could be used as the default store types as well,
>> there's
>> a trade-off I feel regarding the API complexity. Since with this approach,
>> our API complexity granularity is in the order of "number of impl types" *
>> "number of store types". This means that whenever we add new store types
>> in
>> the future, this API needs to be augmented and customized impl needs to be
>> updated to support the new store types, in addition, not all custom impl
>> types may support all store types, but with this interface they are forced
>> to either support all or explicitly throw un-supported exceptions.
>>
>> The way I see a default impl type is that, they would be safe to use for
>> any store types, and since store types are evolved by the library itself,
>> the default impls would better be controlled by the library as well.
>> Custom
>> impl classes can choose to replace some of the stores explicitly, but may
>> not be a best fit as the default impl classes --- if there are in the
>> future, one way we can consider is to make Stores class extensible along
>> with the enum so that advanced users can add more default impls, assuming
>> such scenarios are not very common.
>>
>> So I'm personally still a bit learning towards the enum approach with a
>> narrower scope, for its simplicity as an API and also its low maintenance
>> cost in the future. Let me know what do you think?
>>
>>
>> Guozhang
>>
>>
>> On Wed, Dec 1, 2021 at 6:48 PM Luke Chen  wrote:
>>
>> > Hi devs,
>> >
>> > I'd like to propose a KIP to allow users to set default store
>> > implementation class (built-in RocksDB/InMemory, or custom one), and
>> > default to RocksDB state store, to keep backward compatibility.
>> >
>> > Detailed description can be found here:
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-591%3A+Add+Kafka+Streams+config+to+set+default+state+store
>> >
>> > Any feedback and comments are welcome.
>> >
>> > Thank you.
>> > Luke
>> >
>>
>>
>> --
>> -- Guozhang
>>
>


Re: [VOTE] KIP-806: Add session and window query over KV-store in IQv2

2021-12-15 Thread Luke Chen
gt; > > > > windowFrom, INF: windowTo], and and I was assuming it's now, but
> > otherwise
> > > > > these two are then very much alike.
> > > > >
> > > > > So I guess my question is that, is it by-design to not allow users
> do
> > > > > something like `query.withWindowRange(..).withKeyRange(..)`?
> > > > >
> > > > >
> > > > > >
> > > > > > 3. I got the impression that WindowRangeQuery was intended
> > > > > > for use with both Window and Session stores, but that might
> > > > > > be my assumption.
> > > > > >
> > > > > >
> > > > > Today the session store's query API is at least different on the
> > names,
> > > > > e.g. "findSessions", so I'm not sure if the proposal intend to
> > replace
> > > > > these functions with the WindowRangeQuery such that:
> > > > >
> > > > > sessionStore.findSessions(K, Ins, Ins) ->
> > query.withKey().withWindowRange()
> > > > > sessionStore.findSessions(K, K, Ins, Ins) ->
> > > > > query.withKeyRange().withWindowRange()   // again, assuming we
> would
> > want
> > > > > `withKeyRange` as in 2) above.
> > > > > sessionStore.fetch(K) -> query.withKey()
> > > > > sessionStore.fetch(K, K) -> query.withKeyRange()
> > > > >
> > > > > Also, sessionStore.fetchSession(K, Ins, Ins) seems would not be
> > supported
> > > > > with the new API. I'm not enforcing that we have the complete
> > coverage in
> > > > > this KIP, and I'm with you that we would focus on getting some MVP
> > out
> > > > > sooner, but I'd like our KIP to clarify on what's really covered
> > comparing
> > > > > against the old API and what's not.
> > > > >
> > > > >
> > > > >
> > > > > > 4. That's good feedback. I think those names were inspired
> > > > > > by the WindowStore methods that just say stuff like
> > > > > > `timeFrom` and `timeTo`. The SessionStore does a better job
> > > > > > of being unambiguous, since it says stuff like
> > > > > > `earliestSessionEndTime` and `latestSessionStartTime`.
> > > > > >
> > > > > > And, actually, that brings up a point that I think we
> > > > > > overlooked before. While the method signatures of the
> > > > > > WindowStore and SessionStore methods look the same, the
> > > > > > meanings of their arguments are not the same. In particular,
> > > > > > the WindowStore ranges are always in reference to the window
> > > > > > start time, while the SessionStore ranges may be in
> > > > > > reference to the window start time or end time (or different
> > > > > > combinations of both).
> > > > > >
> > > > > > I suspect that your instinct to cover both stores with the
> > > > > > same Query is correct, and we just need to be specific about
> > > > > > the kinds of ranges we're talking about. For example,
> > > > > > instead of withWindowRange, maybe we would have:
> > > > > >
> > > > > > withWindowStartRange(
> > > > > >   Instant earliestWindowStartTime,
> > > > > >   Instant latestWindowStartTime
> > > > > > );
> > > > > >
> > > > > > Then in the future, we could add stuff like:
> > > > > >
> > > > > > withWindowStartAndEndRange(
> > > > > >   Instant earliestWindowStartTime,
> > > > > >   Instant latestWindowEndTime
> > > > > > );
> > > > > >
> > > > > > Etc.
> > > > > >
> > > > > > 5. I think Luke's question reveals how weirdly similar but
> > > > > > not the same these two queries are. It's not your fault,
> > > > > > this is inherited from the existing set of weirdly-similar-
> > > > > > but-not-the-same store methods. The thing that distinguishes
> > > > > > the WindowKeyQuery from the WindowRangeQuery is:
> > > > > > * WindowKeyQuery: all results share the same K key (cf point
> > > > > > 2: I don't think we need `WindowRangeQuery#withKey(k)`).
> > > > > > Therefore,

Re: When will the version 3.2.0 be released?

2021-12-15 Thread Luke Chen
Hi David,

Here's the release plan page:
https://cwiki.apache.org/confluence/display/KAFKA/Future+release+plan

Currently, we haven't had release date plan for v3.2.0, because v3.1.0 is
still on-going.
But if you check the release interval of each release previously, you can
guess v3.2.0 should be on end of Q1 or early of Q2, 2022.

Thank you.
Luke

On Wed, Dec 15, 2021 at 11:51 PM David Leon 
wrote:

> We have an issue on PROD apparently solved by the issue
> https://issues.apache.org/jira/browse/KAFKA-12980 which will be released
> on the version 3.2.0. When will this version be available to download?
>
> Thanks in advance for the info.
>


[jira] [Resolved] (KAFKA-13545) Workaround for mitigating CVE-2021-4104 Kafka

2021-12-15 Thread Luke Chen (Jira)


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

Luke Chen resolved KAFKA-13545.
---
Resolution: Not A Bug

> Workaround for mitigating CVE-2021-4104 Kafka 
> --
>
> Key: KAFKA-13545
> URL: https://issues.apache.org/jira/browse/KAFKA-13545
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 2.8.1
>Reporter: Akansh Shandilya
>Priority: Major
>
> A new vulnerability is published today :
> https://nvd.nist.gov/vuln/detail/CVE-2021-4104
>  
> Kafka v2.8.1 uses log4j v1.x . Please review following information :
> Is Kafka v2.8.1 impacted by  CVE-2021-4104?
> If yes, is there any workaround/recommendation available for Kafka  v2.8.1 to 
> mitigate CVE-2021-4104



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Reopened] (KAFKA-13545) Workaround for mitigating CVE-2021-4104 Kafka

2021-12-15 Thread Luke Chen (Jira)


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

Luke Chen reopened KAFKA-13545:
---

> Workaround for mitigating CVE-2021-4104 Kafka 
> --
>
> Key: KAFKA-13545
> URL: https://issues.apache.org/jira/browse/KAFKA-13545
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 2.8.1
>Reporter: Akansh Shandilya
>Priority: Major
>
> A new vulnerability is published today :
> https://nvd.nist.gov/vuln/detail/CVE-2021-4104
>  
> Kafka v2.8.1 uses log4j v1.x . Please review following information :
> Is Kafka v2.8.1 impacted by  CVE-2021-4104?
> If yes, is there any workaround/recommendation available for Kafka  v2.8.1 to 
> mitigate CVE-2021-4104



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Resolved] (KAFKA-13545) Workaround for mitigating CVE-2021-4104 Kafka

2021-12-14 Thread Luke Chen (Jira)


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

Luke Chen resolved KAFKA-13545.
---
Resolution: Won't Fix

> Workaround for mitigating CVE-2021-4104 Kafka 
> --
>
> Key: KAFKA-13545
> URL: https://issues.apache.org/jira/browse/KAFKA-13545
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 2.8.1
>Reporter: Akansh Shandilya
>Priority: Major
>
> A new vulnerability is published today :
> https://nvd.nist.gov/vuln/detail/CVE-2021-4104
>  
> Kafka v2.8.1 uses log4j v1.x . Please review following information :
> Is Kafka v2.8.1 impacted by  CVE-2021-4104?
> If yes, is there any workaround/recommendation available for Kafka  v2.8.1 to 
> mitigate CVE-2021-4104



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


Re: [DISCUSS] Enable links from Github commits to Jira Issues

2021-12-14 Thread Luke Chen
Hi Mickael,

I love this idea.
That would be very helpful for reviewers to check the original problem
description in JIRA.

Thank you for raising this!

Luke

On Tue, Dec 14, 2021 at 10:36 PM Mickael Maison  wrote:

> Hi all,
>
> A number of Apache projects have enabled JIRA links in Github commits.
> You can see it in action in the following repositories for example:
> - https://github.com/apache/camel
> - https://github.com/apache/spark
> where the CAMEL-XXX and SPARK-XXX prefixes link to individual JIRAs
>
> I find this feature pretty useful and I propose enabling it on the
> Kafka repository.
> Are there any concerns/objections or are people interested in having this?
>
> Thanks,
> Mickael
>


Re: [VOTE] KIP-806: Add session and window query over KV-store in IQv2

2021-12-13 Thread Luke Chen
Hi Patrick,

Thanks for the KIP!

I have some comments, in addition to Guozhang's comments:
4. The parameter names `windowLower` and `windowUpper` are kind of
ambiguous to me. Could we come up a better name for it, like
`windowStartTime`, `windowEndTime`, or even we don't need the "window"
name, just `startTime` and `endTime`?
5. Why can't we support window range query with a key within a time range?
You might need to explain in the KIP.

Thank you.
Luke


On Sat, Dec 11, 2021 at 7:54 AM Guozhang Wang  wrote:

> Hi Patrick,
>
> I made a pass on the KIP and have a few comments below:
>
> 1. The `WindowRangeQuery` has a private constructor while the
> `WindowKeyQuery` has not, is that intentional?
>
> 2. The `WindowRangeQuery` seems not allowing to range over both window and
> key, but only window with a fixed key, in that case it seems pretty much
> the same as the other (ignoring the constructor), since we know we would
> only have a single `key` value in the returned iterator, and hence it seems
> returning in the form of `WindowStoreIterator` is also fine as the key
> is fixed and hence no need to maintain it in the returned iterator. I'm
> wondering should we actually support ranging over keys as well in
> `WindowRangeQuery`?
>
> 3. The KIP title mentioned both session and window, but the APIs only
> involves window stores; However the return type `WindowStoreIterator` is
> only for window stores not session stores, so I feel we would still have
> some differences for session window query interface?
>
>
> Guozhang
>
> On Fri, Dec 10, 2021 at 1:32 PM Patrick Stuedi
> 
> wrote:
>
> > Hi everyone,
> >
> > I would like to start the vote for KIP-806 that adds window and session
> > query support to query KV-stores using IQv2.
> >
> > The KIP can be found here:
> > https://cwiki.apache.org/confluence/x/LJaqCw
> >
> > Skipping the discussion phase as this KIP is following the same pattern
> as
> > the previously submitted KIP-805 (KIP:
> > https://cwiki.apache.org/confluence/x/85OqCw, Discussion:
> > https://tinyurl.com/msp5mcb2). Of course concerns/comments can still be
> > brought up in this thread.
> >
> > -Patrick
> >
>
>
> --
> -- Guozhang
>


Re: [VOTE] KIP-805: Add range and scan query support in IQ v2

2021-12-13 Thread Luke Chen
Hi Vicky,

I checked the KIP again, and found there's something you might need to have
some description.
The term: *scan query*, is my first time seeing this term.
I think it's not a publicly known term (or it's just me?), and might need
some description for it.
And for the "range query", I think you can also have some words for it.

Thank you.
Luke

On Sat, Dec 11, 2021 at 2:26 AM Guozhang Wang  wrote:

> Thanks Vicky,
>
> I'd suggest we change the KIP title as "add range and scan query over
> kv-store in IQv2" just for clarification, otherwise I'm +1.
>
> Guozhang
>
> On Wed, Dec 8, 2021 at 4:18 PM Matthias J. Sax  wrote:
>
> > Thanks for the KIP.
> >
> > +1 (binding)
> >
> > On 12/5/21 7:03 PM, Luke Chen wrote:
> > > Hi Vasiliki,
> > >
> > > Thanks for the KIP!
> > > It makes sense to have the range and scan query in IQv2, as in IQv1.
> > >
> > > +1 (non-binding)
> > >
> > > Thank you.
> > > Luke
> > >
> > > On Thu, Dec 2, 2021 at 5:41 AM John Roesler 
> wrote:
> > >
> > >> Thanks for the KIP, Vicky!
> > >>
> > >> I’m +1 (binding)
> > >>
> > >> -John
> > >>
> > >> On Tue, Nov 30, 2021, at 14:51, Vasiliki Papavasileiou wrote:
> > >>> Hello everyone,
> > >>>
> > >>> I would like to start a vote for KIP-805 that adds range and scan
> > >> KeyValue
> > >>> queries in IQ2.
> > >>>
> > >>> The KIP can be found here:
> > >>>
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-805%3A+Add+range+and+scan+query+support+in+IQ+v2
> > >>>
> > >>> Cheers!
> > >>> Vicky
> > >>
> > >
> >
>
>
> --
> -- Guozhang
>


Re: [DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-11 Thread Luke Chen
Hi Guozhang,

Thanks for your comments.
I agree that in the KIP, there's a trade-off regarding the API complexity.
With the store impl, we can support default custom stores, but introduce
more complexity for users, while with the enum types, users can configure
default built-in store types easily, but it can't work for custom stores.

For me, I'm OK to narrow down the scope and introduce the default built-in
enum store types first.
And if there's further request, we can consider a better way to support
default store impl.

I'll update the KIP next week, unless there are other opinions from other
members.

Thank you.
Luke

On Fri, Dec 10, 2021 at 6:33 AM Guozhang Wang  wrote:

> Thanks Luke for the updated KIP.
>
> One major change the new proposal has it to change the original enum store
> type with a new interface. Where in the enum approach our internal
> implementations would be something like:
>
> "
> Stores#keyValueBytesStoreSupplier(storeImplTypes, storeName, ...)
> Stores#windowBytesStoreSupplier(storeImplTypes, storeName, ...)
> Stores#sessionBytesStoreSupplier(storeImplTypes, storeName, ...)
> "
>
> And inside the impl classes like here we would could directly do:
>
> "
> if ((supplier = materialized.storeSupplier) == null) {
> supplier =
> Stores.windowBytesStoreSupplier(materialized.storeImplType())
> }
> "
>
> While I understand the benefit of having an interface such that user
> customized stores could be used as the default store types as well, there's
> a trade-off I feel regarding the API complexity. Since with this approach,
> our API complexity granularity is in the order of "number of impl types" *
> "number of store types". This means that whenever we add new store types in
> the future, this API needs to be augmented and customized impl needs to be
> updated to support the new store types, in addition, not all custom impl
> types may support all store types, but with this interface they are forced
> to either support all or explicitly throw un-supported exceptions.
>
> The way I see a default impl type is that, they would be safe to use for
> any store types, and since store types are evolved by the library itself,
> the default impls would better be controlled by the library as well. Custom
> impl classes can choose to replace some of the stores explicitly, but may
> not be a best fit as the default impl classes --- if there are in the
> future, one way we can consider is to make Stores class extensible along
> with the enum so that advanced users can add more default impls, assuming
> such scenarios are not very common.
>
> So I'm personally still a bit learning towards the enum approach with a
> narrower scope, for its simplicity as an API and also its low maintenance
> cost in the future. Let me know what do you think?
>
>
> Guozhang
>
>
> On Wed, Dec 1, 2021 at 6:48 PM Luke Chen  wrote:
>
> > Hi devs,
> >
> > I'd like to propose a KIP to allow users to set default store
> > implementation class (built-in RocksDB/InMemory, or custom one), and
> > default to RocksDB state store, to keep backward compatibility.
> >
> > Detailed description can be found here:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-591%3A+Add+Kafka+Streams+config+to+set+default+state+store
> >
> > Any feedback and comments are welcome.
> >
> > Thank you.
> > Luke
> >
>
>
> --
> -- Guozhang
>


[jira] [Resolved] (KAFKA-13536) Log4J2 Vulnerability zero-day exploit is going on. Will it impact kafka_2.12-2.3.0 version and do we need to upgrade?

2021-12-11 Thread Luke Chen (Jira)


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

Luke Chen resolved KAFKA-13536.
---
Resolution: Duplicate

> Log4J2 Vulnerability zero-day exploit is going on. Will it impact 
> kafka_2.12-2.3.0 version and do we need to upgrade?
> -
>
> Key: KAFKA-13536
> URL: https://issues.apache.org/jira/browse/KAFKA-13536
> Project: Kafka
>  Issue Type: Bug
>Reporter: Rajendra
>Priority: Major
>




--
This message was sent by Atlassian Jira
(v8.20.1#820001)


[jira] [Resolved] (KAFKA-13535) Workaround for mitigating CVE-2021-44228 Kafka

2021-12-11 Thread Luke Chen (Jira)


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

Luke Chen resolved KAFKA-13535.
---
Resolution: Won't Fix

[~akansh] , thanks for reporting the issue. I've confirmed that Kafka is not 
affected by this CVE. Please read my email reply here for more detail: 
[https://lists.apache.org/thread/lgbtvvmy68p0059yoyn9qxzosdmx4jdv] 

Thank you.

> Workaround for mitigating CVE-2021-44228 Kafka 
> ---
>
> Key: KAFKA-13535
> URL: https://issues.apache.org/jira/browse/KAFKA-13535
> Project: Kafka
>  Issue Type: Bug
>Affects Versions: 2.8.1
>Reporter: Akansh Shandilya
>Priority: Major
>
> Kafka v2.8.1 uses log4j v1.x . Please review following information :
>  
> Is Kafka v2.8.1 impacted by  CVE-2021-44228?
> If yes, is there any workaround/recommendation available for Kafka  v2.8.1 to 
> mitigate CVE-2021-44228



--
This message was sent by Atlassian Jira
(v8.20.1#820001)


Re: [DISCUSS] Please review the 3.1.0 blog post

2021-12-11 Thread Luke Chen
Oh, sorry! I have a typo in your name!
Sorry, David! >.<

Luke

On Sat, Dec 11, 2021 at 4:42 PM Luke Chen  wrote:

> Hi Davie,
>
> Thanks for drafting the release announcement post.
> I've checked the content, and looks good to me.
> But I think the header section: "What's New in Apache..." is not formatted
> properly.
> I checked the previous blog post, and it should be a hyperlink just like
> the "Main" kind of font.
>
> [image: image.png]
>
> Thank you.
> Luke
>
>
> On Sat, Dec 11, 2021 at 5:51 AM David Jacot 
> wrote:
>
>> I have put the wrong link in my previous email. Here is the public one:
>>
>> https://blogs.apache.org/preview/kafka/?previewEntry=what-s-new-in-apache7
>>
>> Best,
>> David
>>
>> On Fri, Dec 10, 2021 at 10:35 PM David Jacot  wrote:
>> >
>> > Hello all,
>> >
>> > I have prepared a draft of the release announcement post for the
>> > Apache Kafka 3.1.0 release:
>> >
>> >
>> https://blogs.apache.org/roller-ui/authoring/preview/kafka/?previewEntry=what-s-new-in-apache7
>> >
>> > I would greatly appreciate your reviews if you have a moment.
>> >
>> > Thanks,
>> > David
>>
>


Re: [DISCUSS] Please review the 3.1.0 blog post

2021-12-11 Thread Luke Chen
Hi Davie,

Thanks for drafting the release announcement post.
I've checked the content, and looks good to me.
But I think the header section: "What's New in Apache..." is not formatted
properly.
I checked the previous blog post, and it should be a hyperlink just like
the "Main" kind of font.

[image: image.png]

Thank you.
Luke


On Sat, Dec 11, 2021 at 5:51 AM David Jacot 
wrote:

> I have put the wrong link in my previous email. Here is the public one:
>
> https://blogs.apache.org/preview/kafka/?previewEntry=what-s-new-in-apache7
>
> Best,
> David
>
> On Fri, Dec 10, 2021 at 10:35 PM David Jacot  wrote:
> >
> > Hello all,
> >
> > I have prepared a draft of the release announcement post for the
> > Apache Kafka 3.1.0 release:
> >
> >
> https://blogs.apache.org/roller-ui/authoring/preview/kafka/?previewEntry=what-s-new-in-apache7
> >
> > I would greatly appreciate your reviews if you have a moment.
> >
> > Thanks,
> > David
>


Re: [DISCUSS] KIP-782: Expandable batch size in producer

2021-12-09 Thread Luke Chen
he max
> > record
> > > > size and the max request size, which is unintuitive. Perhaps we could
> > > > introduce a new config max.record.size that defaults to 1MB. We could
> > > then
> > > > increase max.request.size to sth like 10MB.
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > >
> > > > On Mon, Nov 29, 2021 at 6:02 PM Artem Livshits
> > > >  wrote:
> > > >
> > > > > Hi Luke,
> > > > >
> > > > > I don't mind increasing the max.request.size to a higher number,
> e.g.
> > > 2MB
> > > > > could be good.  I think we should also run some benchmarks to see
> the
> > > > > effects of different sizes.
> > > > >
> > > > > I agree that changing round robin to random solves an independent
> > > > existing
> > > > > issue, however the logic in this KIP exacerbates the issue, so
> there
> > is
> > > > > some dependency.
> > > > >
> > > > > -Artem
> > > > >
> > > > > On Wed, Nov 24, 2021 at 12:43 AM Luke Chen 
> > wrote:
> > > > >
> > > > > > Hi Artem,
> > > > > > Yes, I agree if we go with random selection instead of
> round-robin
> > > > > > selection, the latency issue will be more fair. That is, if there
> > are
> > > > 10
> > > > > > partitions, the 10th partition will always be the last choice in
> > each
> > > > > round
> > > > > > in current design, but with random selection, the chance to be
> > > selected
> > > > > is
> > > > > > more fair.
> > > > > >
> > > > > > However, I think that's kind of out of scope with this KIP. This
> is
> > > an
> > > > > > existing issue, and it might need further discussion to decide if
> > > this
> > > > > > change is necessary.
> > > > > >
> > > > > > I agree the default 32KB for "batch.max.size" might be not huge
> > > > > improvement
> > > > > > compared with 256KB. I'm thinking, maybe default to "64KB" for
> > > > > > "batch.max.size", and make the documentation clear that if the
> > > > > > "batch.max.size"
> > > > > > is increased, there might be chances that the "ready" partitions
> > need
> > > > to
> > > > > > wait for next request to send to broker, because of the
> > > > > "max.request.size"
> > > > > > (default 1MB) limitation. "max.request.size" can also be
> considered
> > > to
> > > > > > increase to avoid this issue. What do you think?
> > > > > >
> > > > > > Thank you.
> > > > > > Luke
> > > > > >
> > > > > > On Wed, Nov 24, 2021 at 2:26 AM Artem Livshits
> > > > > >  wrote:
> > > > > >
> > > > > > > >  maybe I can firstly decrease the "batch.max.size" to 32KB
> > > > > > >
> > > > > > > I think 32KB is too small.  With 5 in-flight and 100ms latency
> we
> > > can
> > > > > > > produce 1.6MB/s per partition.  With 256KB we can produce
> > 12.8MB/s
> > > > per
> > > > > > > partition.  We should probably set up some testing and see if
> > 256KB
> > > > has
> > > > > > > problems.
> > > > > > >
> > > > > > > To illustrate latency dynamics, let's consider a simplified
> > model:
> > > 1
> > > > > > > in-flight request per broker, produce latency 125ms, 256KB max
> > > > request
> > > > > > > size, 16 partitions assigned to the same broker, every second
> > 128KB
> > > > is
> > > > > > > produced to each partition (total production rate is 2MB/sec).
> > > > > > >
> > > > > > > If the batch size is 16KB, then the pattern would be the
> > following:
> > > > > > >
> > > > > > > 0ms - produce 128KB into each partition
> > > > > > > 0ms - take 16KB from each partition send (total 256KB)
> > > > > > > 125ms - com

Re: [VOTE] KIP-792: Add "generation" field into consumer protocol

2021-12-09 Thread Luke Chen
Hi Guozhang,

Thanks for your comment.

> we need to make sure the old-versioned leader would be able to ignore the
new
field during an upgrade e.g. without crashing.

Yes, I understand. I'll be careful to make sure it won't crash the old
versioned leader.
But basically, it won't, because we appended the new field into the last of
the ConsumerProtocolSubscription, which means, when read/deserialize the
Subscription metadata, the old versioned leader will just read the head
part of the data.

Thanks for the reminder!

Luke

On Thu, Dec 9, 2021 at 4:00 AM Guozhang Wang  wrote:

> Hi Luke,
>
> Thanks for the KIP.
>
> One thing I'd like to double check is that, since the
> ConsumerProtocolSubscription is not auto generated from the json file, we
> need to make sure the old-versioned leader would be able to ignore the new
> field during an upgrade e.g. without crashing. Other than that, the KIP
> lgtm.
>
> Guozhang
>
> On Tue, Dec 7, 2021 at 6:16 PM Luke Chen  wrote:
>
> > Hi Colin,
> >
> > I'm not quite sure if I understand your thoughts correctly.
> > If I was wrong, please let me know.
> >
> > Also, I'm not quite sure how I could lock this feature to a new IBP
> > version.
> > I saw "KIP-584: Versioning scheme for features" is still under
> development.
> > Not sure if I need to lock the IBP version, how should I do?
> >
> > Thank you.
> > Luke
> >
> > On Tue, Dec 7, 2021 at 9:41 PM Luke Chen  wrote:
> >
> > > Hi Colin,
> > >
> > > Thanks for your comments. I've updated the KIP to mention about the KIP
> > > won't affect current broker side behavior.
> > >
> > > > One scenario that we need to consider is what happens during a
> rolling
> > > upgrade. If the coordinator moves back and forth between brokers with
> > > different IBPs, it seems that the same epoch numbers could be reused
> for
> > a
> > > group, if things are done in the obvious manner (old IBP = don't read
> or
> > > write epoch, new IBP = do)
> > >
> > > I think this KIP doesn't care about the group epoch number at all. The
> > > subscription metadata is passed from each member to group coordinator,
> > and
> > > then the group coordinator pass all of them back to the consumer lead.
> So
> > > even if the epoch number is reused in a group, it should be fine. On
> the
> > > other hand, the group coordinator will have no idea if the join group
> > > request sent from consumer containing the new subscription "generation"
> > > field or not, because group coordinator won't deserialize the metadata.
> > >
> > > I've added also added them into the KIP.
> > >
> > > Thank you.
> > > Luke
> > >
> > > On Mon, Dec 6, 2021 at 10:39 AM Colin McCabe 
> wrote:
> > >
> > >> Hi Luke,
> > >>
> > >> Thanks for the explanation.
> > >>
> > >> I don't see any description of how the broker decides to use the new
> > >> version of ConsumerProtocolSubscription or not. This probably needs to
> > be
> > >> locked to a new IBP version.
> > >>
> > >> One scenario that we need to consider is what happens during a rolling
> > >> upgrade. If the coordinator moves back and forth between brokers with
> > >> different IBPs, it seems that the same epoch numbers could be reused
> > for a
> > >> group, if things are done in the obvious manner (old IBP = don't read
> or
> > >> write epoch, new IBP = do).
> > >>
> > >> best,
> > >> Colin
> > >>
> > >>
> > >> On Fri, Dec 3, 2021, at 18:46, Luke Chen wrote:
> > >> > Hi Colin,
> > >> > Thanks for your comment.
> > >> >
> > >> >> How are we going to avoid the situation where the broker restarts,
> > and
> > >> > the same generation number is reused?
> > >> >
> > >> > Actually, this KIP doesn't have anything to do with the brokers. The
> > >> > "generation" field I added, is in the subscription metadata, which
> > will
> > >> not
> > >> > be deserialized by brokers. The metadata is only deserialized by
> > >> consumer
> > >> > lead. And for the consumer lead, the only thing the lead cared
> about,
> > is
> > >> > the highest generation of the ownedPartitions among all the
> consumers.
> > >> With
> > >> > the highest generation of th

Re: AccessDeniedException Kafka 3.0.0 on Windows

2021-12-09 Thread Luke Chen
Hi Martijn,

The v3.1.0 should be released soon. It's been code freezed, and we're
fixing some blocker issues now. FYI

Thank you.
Luke

On Thu, Dec 9, 2021 at 5:36 PM de Bruijn, M. (Martijn)
 wrote:

> Thanks Luke for your reply. It's good to see its fixed already. When will
> it be released?
> Regards,
> Martijn
>
> -Original Message-
> From: Luke Chen 
> Sent: donderdag 9 december 2021 10:30
> To: dev 
> Subject: Re: AccessDeniedException Kafka 3.0.0 on Windows
>
> Hi Martijn de Bruijn
>
> Thanks for reporting the issue. This is a known issue and it will be fixed
> in V3.0.1/V3.1.0 in this ticket:
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FKAFKA-13391data=04%7C01%7Cmartijn.de.bruijn%40befrank.nl%7C53ac2de23e694da1e29308d9baf67b79%7Cfed95e698d7343feaffba7d85ede36fb%7C1%7C0%7C637746390798820561%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=b2sntcAZCKqfgMLW5GWT5g%2BDDckm6ourZc6z%2FgGNo9k%3Dreserved=0
> .
>
> FYI
>
> Thank you.
> Luke
>
>
>
> On Thu, Dec 9, 2021 at 4:08 PM de Bruijn, M. (Martijn) <
> martijn.de.bru...@befrank.nl.invalid> wrote:
>
> > On Windows, Kafka 3.0.0 is producing AccessDeniedException's. Mainly
> > due to using FileChannel.open(path, StandardOpenOption.READ) on a
> > directory which will always fail on Windows.
> > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
> > l.openjdk.java.net%2Fpipermail%2Fnio-dev%2F2013-February%2F002123.html
> > data=04%7C01%7Cmartijn.de.bruijn%40befrank.nl%7C53ac2de23e694da1e
> > 29308d9baf67b79%7Cfed95e698d7343feaffba7d85ede36fb%7C1%7C0%7C637746390
> > 798820561%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIi
> > LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=qeF0DvzJiw7jaIKERQhyJL
> > FYnBD4pOie7PyHesCk4kY%3Dreserved=0
> > )
> > Example error code:
> > org.apache.kafka.common.utils.Utils.flushDir(Utils.java:953)
> >
> > Any change this will be solved soon?
> >
> > Spring Boot 2.6 upgraded the Kafka dependency to 3.0.0 so this error
> > has high impact as all tests using @EmbeddedKafka on Windows developer
> > laptops won't work anymore.
> >
> > More error reports:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstac
> > koverflow.com%2Fquestions%2F69289641%2Faccessdeniedexception-while-run
> > ning-apache-kafka-3-on-windowsdata=04%7C01%7Cmartijn.de.bruijn%40
> > befrank.nl%7C53ac2de23e694da1e29308d9baf67b79%7Cfed95e698d7343feaffba7
> > d85ede36fb%7C1%7C0%7C637746390798820561%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> > p;sdata=%2B7ew95wdsjsFxHC%2FYdpEvNnwCoMNTI8H81Ksi3S175o%3Dreserve
> > d=0
> >
> >
> >
> > Regards,
> > Martijn de Bruijn
> > Software engineer
> >
>


Re: AccessDeniedException Kafka 3.0.0 on Windows

2021-12-09 Thread Luke Chen
Hi Martijn de Bruijn

Thanks for reporting the issue. This is a known issue and it will be fixed
in V3.0.1/V3.1.0 in this ticket:
https://issues.apache.org/jira/browse/KAFKA-13391.

FYI

Thank you.
Luke



On Thu, Dec 9, 2021 at 4:08 PM de Bruijn, M. (Martijn)
 wrote:

> On Windows, Kafka 3.0.0 is producing AccessDeniedException's. Mainly due
> to using FileChannel.open(path, StandardOpenOption.READ) on a directory
> which will always fail on Windows.
> (https://mail.openjdk.java.net/pipermail/nio-dev/2013-February/002123.html
> )
> Example error code:
> org.apache.kafka.common.utils.Utils.flushDir(Utils.java:953)
>
> Any change this will be solved soon?
>
> Spring Boot 2.6 upgraded the Kafka dependency to 3.0.0 so this error has
> high impact as all tests using @EmbeddedKafka on Windows developer laptops
> won't work anymore.
>
> More error reports:
> https://stackoverflow.com/questions/69289641/accessdeniedexception-while-running-apache-kafka-3-on-windows
>
>
>
> Regards,
> Martijn de Bruijn
> Software engineer
>


Re: [VOTE] KIP-792: Add "generation" field into consumer protocol

2021-12-07 Thread Luke Chen
Hi Colin,

I'm not quite sure if I understand your thoughts correctly.
If I was wrong, please let me know.

Also, I'm not quite sure how I could lock this feature to a new IBP
version.
I saw "KIP-584: Versioning scheme for features" is still under development.
Not sure if I need to lock the IBP version, how should I do?

Thank you.
Luke

On Tue, Dec 7, 2021 at 9:41 PM Luke Chen  wrote:

> Hi Colin,
>
> Thanks for your comments. I've updated the KIP to mention about the KIP
> won't affect current broker side behavior.
>
> > One scenario that we need to consider is what happens during a rolling
> upgrade. If the coordinator moves back and forth between brokers with
> different IBPs, it seems that the same epoch numbers could be reused for a
> group, if things are done in the obvious manner (old IBP = don't read or
> write epoch, new IBP = do)
>
> I think this KIP doesn't care about the group epoch number at all. The
> subscription metadata is passed from each member to group coordinator, and
> then the group coordinator pass all of them back to the consumer lead. So
> even if the epoch number is reused in a group, it should be fine. On the
> other hand, the group coordinator will have no idea if the join group
> request sent from consumer containing the new subscription "generation"
> field or not, because group coordinator won't deserialize the metadata.
>
> I've added also added them into the KIP.
>
> Thank you.
> Luke
>
> On Mon, Dec 6, 2021 at 10:39 AM Colin McCabe  wrote:
>
>> Hi Luke,
>>
>> Thanks for the explanation.
>>
>> I don't see any description of how the broker decides to use the new
>> version of ConsumerProtocolSubscription or not. This probably needs to be
>> locked to a new IBP version.
>>
>> One scenario that we need to consider is what happens during a rolling
>> upgrade. If the coordinator moves back and forth between brokers with
>> different IBPs, it seems that the same epoch numbers could be reused for a
>> group, if things are done in the obvious manner (old IBP = don't read or
>> write epoch, new IBP = do).
>>
>> best,
>> Colin
>>
>>
>> On Fri, Dec 3, 2021, at 18:46, Luke Chen wrote:
>> > Hi Colin,
>> > Thanks for your comment.
>> >
>> >> How are we going to avoid the situation where the broker restarts, and
>> > the same generation number is reused?
>> >
>> > Actually, this KIP doesn't have anything to do with the brokers. The
>> > "generation" field I added, is in the subscription metadata, which will
>> not
>> > be deserialized by brokers. The metadata is only deserialized by
>> consumer
>> > lead. And for the consumer lead, the only thing the lead cared about, is
>> > the highest generation of the ownedPartitions among all the consumers.
>> With
>> > the highest generation of the ownedPartitions, the consumer lead can
>> > distribute the partitions as sticky as possible, and most importantly,
>> > without errors.
>> >
>> > That is, after this KIP, if the broker restarts, and the same generation
>> > number is reused, it won't break current rebalance behavior. But it'll
>> help
>> > the consumer lead do the sticky assignments correctly.
>> >
>> > Thank you.
>> > Luke
>> >
>> > On Fri, Dec 3, 2021 at 6:30 AM Colin McCabe  wrote:
>> >
>> >> How are we going to avoid the situation where the broker restarts, and
>> the
>> >> same generation number is reused?
>> >>
>> >> best,
>> >> Colin
>> >>
>> >> On Tue, Nov 30, 2021, at 16:36, Luke Chen wrote:
>> >> > Hi all,
>> >> >
>> >> > I'd like to start the vote for KIP-792: Add "generation" field into
>> >> > consumer protocol.
>> >> >
>> >> > The goal of this KIP is to allow the assignor/consumer coordinator to
>> >> have
>> >> > a way to identify the out-of-date members/assignments, to avoid
>> rebalance
>> >> > stuck issues in current protocol.
>> >> >
>> >> > Detailed description can be found here:
>> >> >
>> >>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614
>> >> >
>> >> > Any feedback is welcome.
>> >> >
>> >> > Thank you.
>> >> > Luke
>> >>
>>
>


Re: [VOTE] KIP-792: Add "generation" field into consumer protocol

2021-12-07 Thread Luke Chen
Hi Colin,

Thanks for your comments. I've updated the KIP to mention about the KIP
won't affect current broker side behavior.

> One scenario that we need to consider is what happens during a rolling
upgrade. If the coordinator moves back and forth between brokers with
different IBPs, it seems that the same epoch numbers could be reused for a
group, if things are done in the obvious manner (old IBP = don't read or
write epoch, new IBP = do)

I think this KIP doesn't care about the group epoch number at all. The
subscription metadata is passed from each member to group coordinator, and
then the group coordinator pass all of them back to the consumer lead. So
even if the epoch number is reused in a group, it should be fine. On the
other hand, the group coordinator will have no idea if the join group
request sent from consumer containing the new subscription "generation"
field or not, because group coordinator won't deserialize the metadata.

I've added also added them into the KIP.

Thank you.
Luke

On Mon, Dec 6, 2021 at 10:39 AM Colin McCabe  wrote:

> Hi Luke,
>
> Thanks for the explanation.
>
> I don't see any description of how the broker decides to use the new
> version of ConsumerProtocolSubscription or not. This probably needs to be
> locked to a new IBP version.
>
> One scenario that we need to consider is what happens during a rolling
> upgrade. If the coordinator moves back and forth between brokers with
> different IBPs, it seems that the same epoch numbers could be reused for a
> group, if things are done in the obvious manner (old IBP = don't read or
> write epoch, new IBP = do).
>
> best,
> Colin
>
>
> On Fri, Dec 3, 2021, at 18:46, Luke Chen wrote:
> > Hi Colin,
> > Thanks for your comment.
> >
> >> How are we going to avoid the situation where the broker restarts, and
> > the same generation number is reused?
> >
> > Actually, this KIP doesn't have anything to do with the brokers. The
> > "generation" field I added, is in the subscription metadata, which will
> not
> > be deserialized by brokers. The metadata is only deserialized by consumer
> > lead. And for the consumer lead, the only thing the lead cared about, is
> > the highest generation of the ownedPartitions among all the consumers.
> With
> > the highest generation of the ownedPartitions, the consumer lead can
> > distribute the partitions as sticky as possible, and most importantly,
> > without errors.
> >
> > That is, after this KIP, if the broker restarts, and the same generation
> > number is reused, it won't break current rebalance behavior. But it'll
> help
> > the consumer lead do the sticky assignments correctly.
> >
> > Thank you.
> > Luke
> >
> > On Fri, Dec 3, 2021 at 6:30 AM Colin McCabe  wrote:
> >
> >> How are we going to avoid the situation where the broker restarts, and
> the
> >> same generation number is reused?
> >>
> >> best,
> >> Colin
> >>
> >> On Tue, Nov 30, 2021, at 16:36, Luke Chen wrote:
> >> > Hi all,
> >> >
> >> > I'd like to start the vote for KIP-792: Add "generation" field into
> >> > consumer protocol.
> >> >
> >> > The goal of this KIP is to allow the assignor/consumer coordinator to
> >> have
> >> > a way to identify the out-of-date members/assignments, to avoid
> rebalance
> >> > stuck issues in current protocol.
> >> >
> >> > Detailed description can be found here:
> >> >
> >>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614
> >> >
> >> > Any feedback is welcome.
> >> >
> >> > Thank you.
> >> > Luke
> >>
>


Re: [VOTE] KIP-805: Add range and scan query support in IQ v2

2021-12-05 Thread Luke Chen
Hi Vasiliki,

Thanks for the KIP!
It makes sense to have the range and scan query in IQv2, as in IQv1.

+1 (non-binding)

Thank you.
Luke

On Thu, Dec 2, 2021 at 5:41 AM John Roesler  wrote:

> Thanks for the KIP, Vicky!
>
> I’m +1 (binding)
>
> -John
>
> On Tue, Nov 30, 2021, at 14:51, Vasiliki Papavasileiou wrote:
> > Hello everyone,
> >
> > I would like to start a vote for KIP-805 that adds range and scan
> KeyValue
> > queries in IQ2.
> >
> > The KIP can be found here:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-805%3A+Add+range+and+scan+query+support+in+IQ+v2
> >
> > Cheers!
> > Vicky
>


Re: [VOTE] KIP-799: Align behaviour for producer callbacks with documented behaviour

2021-12-03 Thread Luke Chen
Hi Séamus,

Thanks for the update.
Looks better now!

Thank you.
Luke

On Sat, Dec 4, 2021 at 12:57 AM Séamus Ó Ceanainn <
seamus.oceana...@zalando.ie> wrote:

> Hey Luke,
>
> Thanks for the feedback. I've updated the relevant section to hopefully
> make it more clear from the KIP itself what placeholder value would be
> returned.
>
> Regards,
> Séamus.
>
> On Tue, 30 Nov 2021 at 09:52, Luke Chen  wrote:
>
> > Hi Séamus,
> > Thanks for the KIP!
> > We definitely want to keep the producer callback consistent for all types
> > of errors.
> >
> > Just one comment for the KIP:
> > In the "Proposed Changes" section, could you please "explicitly" describe
> > what placeholder you'll return, in addition to adding a hyperlink to
> other
> > places, to make it clear.
> >
> > +1 (non-binding)
> >
> > Thank you.
> > Luke
> >
> > On Tue, Nov 30, 2021 at 1:17 PM John Roesler 
> wrote:
> >
> > > Thanks, Séamus!
> > >
> > > I'm +1 (binding).
> > >
> > > On Mon, 2021-11-29 at 16:14 +, Séamus Ó Ceanainn wrote:
> > > > Hi everyone,
> > > >
> > > > I'd like to start a vote for KIP-799: Align behaviour for producer
> > > > callbacks with documented behaviour
> > > > <
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-799%3A+Align+behaviour+for+producer+callbacks+with+documented+behaviour
> > > >
> > > > .
> > > >
> > > > The KIP proposes a breaking change in the behaviour of producer
> client
> > > > callbacks. The breaking change would align the behaviour of callbacks
> > > with
> > > > the documented behaviour for the method, and makes it consistent with
> > > > similar methods for producer client interceptors.
> > > >
> > > > Regards,
> > > > Séamus.
> > >
> > >
> >
>


Re: [VOTE] KIP-792: Add "generation" field into consumer protocol

2021-12-03 Thread Luke Chen
Hi Colin,
Thanks for your comment.

> How are we going to avoid the situation where the broker restarts, and
the same generation number is reused?

Actually, this KIP doesn't have anything to do with the brokers. The
"generation" field I added, is in the subscription metadata, which will not
be deserialized by brokers. The metadata is only deserialized by consumer
lead. And for the consumer lead, the only thing the lead cared about, is
the highest generation of the ownedPartitions among all the consumers. With
the highest generation of the ownedPartitions, the consumer lead can
distribute the partitions as sticky as possible, and most importantly,
without errors.

That is, after this KIP, if the broker restarts, and the same generation
number is reused, it won't break current rebalance behavior. But it'll help
the consumer lead do the sticky assignments correctly.

Thank you.
Luke

On Fri, Dec 3, 2021 at 6:30 AM Colin McCabe  wrote:

> How are we going to avoid the situation where the broker restarts, and the
> same generation number is reused?
>
> best,
> Colin
>
> On Tue, Nov 30, 2021, at 16:36, Luke Chen wrote:
> > Hi all,
> >
> > I'd like to start the vote for KIP-792: Add "generation" field into
> > consumer protocol.
> >
> > The goal of this KIP is to allow the assignor/consumer coordinator to
> have
> > a way to identify the out-of-date members/assignments, to avoid rebalance
> > stuck issues in current protocol.
> >
> > Detailed description can be found here:
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614
> >
> > Any feedback is welcome.
> >
> > Thank you.
> > Luke
>


[DISCUSS] KIP-591: Add Kafka Streams config to set default state store

2021-12-01 Thread Luke Chen
Hi devs,

I'd like to propose a KIP to allow users to set default store
implementation class (built-in RocksDB/InMemory, or custom one), and
default to RocksDB state store, to keep backward compatibility.

Detailed description can be found here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-591%3A+Add+Kafka+Streams+config+to+set+default+state+store

Any feedback and comments are welcome.

Thank you.
Luke


[VOTE] KIP-792: Add "generation" field into consumer protocol

2021-11-30 Thread Luke Chen
Hi all,

I'd like to start the vote for KIP-792: Add "generation" field into
consumer protocol.

The goal of this KIP is to allow the assignor/consumer coordinator to have
a way to identify the out-of-date members/assignments, to avoid rebalance
stuck issues in current protocol.

Detailed description can be found here:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614

Any feedback is welcome.

Thank you.
Luke


Re: [VOTE] KIP-799: Align behaviour for producer callbacks with documented behaviour

2021-11-30 Thread Luke Chen
Hi Séamus,
Thanks for the KIP!
We definitely want to keep the producer callback consistent for all types
of errors.

Just one comment for the KIP:
In the "Proposed Changes" section, could you please "explicitly" describe
what placeholder you'll return, in addition to adding a hyperlink to other
places, to make it clear.

+1 (non-binding)

Thank you.
Luke

On Tue, Nov 30, 2021 at 1:17 PM John Roesler  wrote:

> Thanks, Séamus!
>
> I'm +1 (binding).
>
> On Mon, 2021-11-29 at 16:14 +, Séamus Ó Ceanainn wrote:
> > Hi everyone,
> >
> > I'd like to start a vote for KIP-799: Align behaviour for producer
> > callbacks with documented behaviour
> > <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-799%3A+Align+behaviour+for+producer+callbacks+with+documented+behaviour
> >
> > .
> >
> > The KIP proposes a breaking change in the behaviour of producer client
> > callbacks. The breaking change would align the behaviour of callbacks
> with
> > the documented behaviour for the method, and makes it consistent with
> > similar methods for producer client interceptors.
> >
> > Regards,
> > Séamus.
>
>


Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-28 Thread Luke Chen
Hi devs,
Welcome to provide feedback.

If there are no other comments, I'll start a vote tomorrow.

Thank you.
Luke


On Mon, Nov 22, 2021 at 4:16 PM Luke Chen  wrote:

> Hello David,
>
> For (3):
>
>
>
> * I suppose that we could add a `generation` field to the JoinGroupRequest
> instead to do the fencing that you describe while handling the sentinel in
> the assignor directly. If we would add the `generation` to the request
> itself, would we need the `generation` in the subscription protocol as
> well?*
>
> On second thought, I think this is not better than adding `generation`
> field in the subscription protocol, because I think we don't have to do any
> generation validation on joinGroup request. The purpose of
> `joinGroupRequest` is to accept any members to join this group, even if the
> member is new or ever joined or what. As long as we have the generationId
> in the subscription metadata, the consumer lead can leverage the info to
> ignore the old ownedPartitions (or do other handling), and the rebalance
> can still complete successfully and correctly. On the other hand, if we did
> the generation check on JoinGroupRequest, and return `ILLEGAL_GENERATION`
> back to consumer, the consumer needs to clear its generation info and
> rejoin the group to continue the rebalance. It needs more request/response
> network and slow down the rebalance.
>
> So I think we should add the `generationId` field into Subscription
> protocol to achieve what we want.
>
> Thank you.
> Luke
>
> On Thu, Nov 18, 2021 at 8:51 PM Luke Chen  wrote:
>
>> Hi David,
>> Thanks for your feedback.
>>
>> I've updated the KIP for your comments (1)(2).
>> For (3), it's a good point! Yes, we didn't deserialize the subscription
>> metadata on broker side, and it's not necessary to add overhead on broker
>> side. And, yes, I think we can fix the original issue by adding a
>> "generation" field into `JoinGroupRequest` instead, and also add a field
>> into `JoinGroupResponse` in `JoinGroupResponseMember` field. That way, the
>> broker can identify the old member from `JoinGroupRequest`. And the
>> assignor can also get the "generation" info via the `Subscription` instance.
>>
>> I'll update the KIP to add "generation" field into `JoinGroupRequest` and
>> `JoinGroupResponse`, if there is no other options.
>>
>> Thank you.
>> Luke
>>
>>
>> On Tue, Nov 16, 2021 at 12:31 AM David Jacot 
>> wrote:
>>
>>> Hi Luke,
>>>
>>> Thanks for the KIP. Overall, I think that the motivation makes sense. I
>>> have a couple of comments/questions:
>>>
>>> 1. In the Public Interfaces section, it would be great if you could put
>>> the
>>> end state not the current one.
>>>
>>> 2. Do we need to update the Subscription class to expose the
>>> generation? If so, it would be great to mention it in the Public
>>> Interfaces section as well.
>>>
>>> 3. You mention that the broker will set the generation if the
>>> subscription
>>> contains a sentinel value (-1). As of today, the broker does not parse
>>> the subscription so I am not sure how/why we would do this. I suppose
>>> that we could add a `generation` field to the JoinGroupRequest instead
>>> to do the fencing that you describe while handling the sentinel in the
>>> assignor directly. If we would add the `generation` to the request
>>> itself,
>>> would we need the `generation` in the subscription protocol as well?
>>>
>>> Best,
>>> David
>>>
>>> On Fri, Nov 12, 2021 at 3:31 AM Luke Chen  wrote:
>>> >
>>> > Hi all,
>>> >
>>> > I'd like to start the discussion for KIP-792: Add "generation" field
>>> into
>>> > consumer protocol.
>>> >
>>> > The goal of this KIP is to allow assignor/consumer coordinator/group
>>> > coordinator to have a way to identify the out-of-date
>>> members/assignments.
>>> >
>>> > Detailed description can be found here:
>>> >
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614
>>> >
>>> > Any feedback is welcome.
>>> >
>>> > Thank you.
>>> > Luke
>>>
>>


Re: Handling retriable exceptions during Connect source task start

2021-11-28 Thread Luke Chen
Hi Gunnar and Sergei,
I think it's good to have a retriable exception handling during task#start.

> are retriable exceptions during start disallowed by
design, and the task must not throw retriable exceptions during start, or
it's just currently not supported by the Connect framework and I just need
to implement proper error handling in the connector?

> Would it require a KIP?

Sorry, I'm not sure if it's by design or not supported and needed to be
implemented.
But I guess if you want to implement the error handling in connector, you
might leverage existing retry configuration, or you'll create a new one.
Either way, I think it needs a small KIP as you mentioned, task#start is
not covered in KIP-298. On the other hand, I think having a KIP first is
good, to make sure you're on the right track, before you get your hand
dirty. Besides, KIP discussion would have more attention, I think. :)

Thank you.
Luke

On Fri, Nov 26, 2021 at 4:09 PM Gunnar Morling
 wrote:

> Hi all,
>
> We encountered a similar situation in Debezium again, where an exception
> during Task::start() would be desirable to be retried.
>
> Would anything speak against implementing retriable support for
> Task::start() in Kafka Connect? Would it require a KIP?
>
> Thanks,
>
> --Gunnar
>
>
> Am Mo., 9. Aug. 2021 um 10:47 Uhr schrieb Gunnar Morling <
> gunnar.morl...@googlemail.com>:
>
> > Hi,
> >
> > To ask slightly differently: would there be interest in a pull request
> for
> > implementing retries, in case RetriableException is thrown from the
> > Task::start() method?
> >
> > Thanks,
> >
> > --Gunnar
> >
> >
> > Am Do., 5. Aug. 2021 um 22:27 Uhr schrieb Sergei Morozov  >:
> >
> >> Hi,
> >>
> >> I'm trying to address an issue in Debezium (DBZ-3823
> >> ) where a source connector
> >> task
> >> cannot recover from a retriable exception.
> >>
> >> The root cause is that the task interacts with the source database
> during
> >> SourceTask#start but Kafka Connect doesn't handle retriable exceptions
> >> thrown at this stage as retriable. KIP-298
> >> <
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-298%3A+Error+Handling+in+Connect
> >> >
> >> that
> >> originally introduced handling of retriable exception doesn't describe
> >> handling task start exceptions, so it's unclear to me whether those
> aren't
> >> allowed by design or it was just out of the scope of the KIP.
> >>
> >> My current working solution
> >>  relies
> >> on the internal Debezium implementation of the task restart which
> >> introduces certain risks (the details are in the PR description).
> >>
> >> The question is: are retriable exceptions during start disallowed by
> >> design, and the task must not throw retriable exceptions during start,
> or
> >> it's just currently not supported by the Connect framework and I just
> need
> >> to implement proper error handling in the connector?
> >>
> >> Thanks!
> >>
> >> --
> >> Sergei Morozov
> >>
> >
>


Re: [DISCUSS] KIP-753: ACL authentication, Host field support IP network segment

2021-11-26 Thread Luke Chen
Hi Lobo,
Thanks for the KIP!

I like the idea to allow "IP subnet" to be passed into `--allow-host`
option to set for a principle. It will be useful in production environment.

Here's some comments:
1. I think "IP subnet" is more specific than "network segment", is that
right?
2. Since you allow the IP subnet in "--allow-host" option, should we also
allow the IP subnet in "--deny-host" option?
3. You should mention that we only accept the "CIDR notation" of the IP
subnet, to avoid other kinds of subnet expression. REF:
https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing#CIDR_notation
4. IP subnet also supports IPv6, should we also allow subnet of IPv6?

Thank you.
Luke

On Tue, Jun 8, 2021 at 9:19 AM lobo xu  wrote:

> The KIP address is wrong in the last email. This is the correct Kip Wiki
> address
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-753%3A++ACL+authentication%2C+Host+field+support+IP+network+segment
>
>
> On 2021/06/07 16:24:50, lobo xu  wrote:
> > Hi all
> >
> > I'd like to discuss the following kip, any suggestions are welcome.
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-753%3A++ACL+authentication%2C+Host+field+support+IP+network+segment
> 。
> >
> > Many thanks,
> >
> > Lobo
> >
>


Re: KIP-800: Add reason to LeaveGroupRequest

2021-11-25 Thread Luke Chen
Hi David,
Thanks for the update.

Just voted! :)

Thank you.
Luke

On Thu, Nov 25, 2021 at 8:42 PM David Jacot 
wrote:

> Hi Luke,
>
> Good point. I have renamed the KIP.
>
> Thanks,
> David
>
> On Thu, Nov 25, 2021 at 4:28 AM Luke Chen  wrote:
> >
> > Hi David,
> > Sorry for the late reply.
> > Thanks for the update. It looks good now.
> > I love the idea to add joinGroup request reason, as well as the
> > `enforceRebalance` API!
> >
> > One minor comment, since we extended the original KIP from leaveGroup to
> > joinGroup request, Could you please update the KIP title and also the
> > motivation section?
> > It makes the KIP much clearer for future reference.
> >
> > Otherwise, LGTM!
> >
> > Thank you.
> > Luke
> >
> > On Fri, Nov 19, 2021 at 11:23 PM David Jacot  >
> > wrote:
> >
> > > I have updated the KIP.
> > >
> > > Best,
> > > David
> > >
> > > On Fri, Nov 19, 2021 at 3:00 PM David Jacot 
> wrote:
> > > >
> > > > Thank you all for your feedback. Let me address all your points
> below.
> > > >
> > > > Luke,
> > > > 1. I use a tag field so bumping the request version is not
> necessary. In
> > > > this case, using a tag field does not seem to be the best approach so
> > > > I will use a regular one and bump the version.
> > > > 2. Sounds good. I will fix that comment.
> > > > 3. That is a good question. My intent was to avoid getting weird or
> > > cryptic
> > > > reasons logged on the broker so I thought that having a standard one
> is
> > > > better. As Sophie suggested something similar for the
> `enforceRebalance`
> > > > API, we could do it for both, I suppose.
> > > >
> > > > Ismael,
> > > > 1. That's a good point. I chose to use a tag field to avoid having to
> > > bump
> > > > the request version. In this particular case, it seems that bumping
> the
> > > > version does not cost much so it is perhaps better. I will change
> this.
> > > >
> > > > Sophie,
> > > > 1. That's a pretty good idea, thanks. Let me update the KIP to
> include
> > > > a reason in the JoinGroup request. Regarding the Consumer API, do
> > > > you think that there is value for KStreams to expose the possibility
> to
> > > > provide a reason? Otherwise, it might be better to use a default
> > > > reason in this case.
> > > > 2. I don't fully get your point about providing the reason to the
> group
> > > > leader assignor on the client. Do you think that we should propagate
> > > > it to all the consumers or to the leader as well? The user usually
> has
> > > > access to all its client logs so I am not sure that it is really
> > > necessary.
> > > > Could you elaborate?
> > > >
> > > > I will update the KIP soon.
> > > >
> > > > Best,
> > > > David
> > > >
> > > > On Sat, Nov 13, 2021 at 6:00 AM Sophie Blee-Goldman
> > > >  wrote:
> > > > >
> > > > > This sounds great, thanks David.
> > > > >
> > > > > One thought: what do you think about doing something similar for
> the
> > > > > JoinGroup request?
> > > > >
> > > > > When you only have broker logs and not client logs, it's somewhere
> > > between
> > > > > challenging and
> > > > > impossible to determine the reason for a rebalance that was
> triggered
> > > > > explicitly by the client or
> > > > > even the user. For example, when a followup rebalance is requested
> to
> > > > > assign the revoked
> > > > > partitions after a cooperative rebalance. Or any of the many
> reasons we
> > > > > trigger a rebalance
> > > > > in Kafka Streams, via the #enforceRebalance API.
> > > > >
> > > > > Perhaps we could add a parameter to that method as such:
> > > > >
> > > > > public void enforceRebalance(final String reason);
> > > > >
> > > > > Then we can add that to the JoinGroup request/ConsumerProtocol. Not
> > > only
> > > > > would it help to
> > > > > log this reason on the broker side, the information about who
> > > requested the
> > > > > rebalance and why
> > > > > could be extremely useful to have available to the group
> > > leader/partition
> > > > > assignor on the client
> > > > > side.
> > > > >
> > > > > Cheers,
> > > > > Sophie
> > > > >
> > > > > On Fri, Nov 12, 2021 at 10:05 AM Ismael Juma 
> > > wrote:
> > > > >
> > > > > > Thanks David, this is a worthwhile improvement. Quick question,
> why
> > > did we
> > > > > > pick a tagged field here?
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Thu, Nov 11, 2021, 8:32 AM David Jacot
> > > 
> > > > > > wrote:
> > > > > >
> > > > > > > Hi folks,
> > > > > > >
> > > > > > > I'd like to discuss this very small KIP which proposes to add a
> > > reason
> > > > > > > field
> > > > > > > to the LeaveGroupRequest in order to let the broker know why a
> > > member
> > > > > > > left the group. This would be really handy for administrators.
> > > > > > >
> > > > > > > KIP: https://cwiki.apache.org/confluence/x/eYyqCw
> > > > > > >
> > > > > > > Cheers,
> > > > > > > David
> > > > > > >
> > > > > >
> > >
>


Re: [VOTE] KIP-800: Add reason to LeaveGroupRequest

2021-11-25 Thread Luke Chen
Hi David,
Thanks for the KIP!
It is good to have the joinGroup/leaveGroup reason sent to brokers for
better troubleshooting.

+1 (non-binding)

Thank you.
Luke

On Thu, Nov 25, 2021 at 8:14 AM Gwen Shapira 
wrote:

> +1
>
> Thanks for driving David. Super useful.
>
> On Wed, Nov 24, 2021 at 8:53 AM David Jacot 
> wrote:
>
> > Hi folks,
> >
> > I'd like to start a vote on KIP-800: Add reason to LeaveGroupRequest.
> >
> > KIP: https://cwiki.apache.org/confluence/x/eYyqCw
> >
> > Please let me know what you think.
> >
> > Best,
> > David
> >
>
>
> --
> Gwen Shapira
> Engineering Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter | blog
>


Re: Errors thrown from a KStream transformer are swallowed, eg. StackOverflowError

2021-11-24 Thread Luke Chen
Yes, I confirmed that in KAFKA-9331 (
https://github.com/apache/kafka/pull/9487), we removed the `catch
(exception)` block for adding uncaught exception handler.
Then, in KAFKA-12537(https://github.com/apache/kafka/pull/10387), we added
`catch (Thrwoable)` (in v2.8.0).

So, @Sinclair, if you upgrade to v2.8.0 or later, this issue should be
fixed.

Thank you.
Luke





On Sat, Nov 20, 2021 at 8:49 AM Matthias J. Sax  wrote:

> Not sure what version you are using, but it say `Thrwoable` in `trunk`
>
>
> https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java#L577
>
>
> -Matthias
>
> On 11/18/21 6:09 AM, John Roesler wrote:
> > Thanks for pointing that out, Scott!
> >
> > You’re totally right; that should be a Throwable.
> >
> > Just to put it out there, do you want to just send a quick PR? If not,
> no worries. I’m just asking because it seems like you’ve already done the
> hard part and it might be nice to get the contribution credit.
> >
> > Thanks,
> > John
> >
> > On Thu, Nov 18, 2021, at 08:00, Sinclair Scott wrote:
> >> Hi there,
> >>
> >>
> >> I'm a big fan of KStreams - thanks for all the great work!!
> >>
> >>
> >> I unfortunately (my fault) had a StackOverflowError bug in my KStream
> >> transformer which meant that the KStream died without reporting any
> >> Exception at all.
> >>
> >>
> >> The first log message showed some polling activity and then you see
> >> later the State transition to PENDING_SHUTDOWN
> >>
> >>
> >> Main Consumer poll completed in 2 ms and fetched 1 records
> >> Flushing all global globalStores registered in the state manager
> >> Idempotently invoking restoration logic in state RUNNING
> >> Finished restoring all changelogs []
> >> Idempotent restore call done. Thread state has not changed.
> >> Processing tasks with 1 iterations.
> >> Flushing all global globalStores registered in the state manager
> >> State transition from RUNNING to PENDING_SHUTDOWN
> >>
> >>
> >>
> >> This is because the StreamThread.run() method catches Exception only.
> >>
> >>
> >> I ended up recompiling the kstreams and changing the catch to Throwable
> >> so I can see what was going on. Then I discovered my bad recursive call
> >>   :(
> >>
> >>
> >> Can we please change the Catch to catch Throwable , so that we are
> >> always guaranteed some output?
> >>
> >>
> >> StreamThread.java
> >>
> >> @Override
> >> public void run() {
> >>  log.info("Starting");
> >>  if (setState(State.STARTING) == null) {
> >>  log.info("StreamThread already shutdown. Not running");
> >>  return;
> >>  }
> >>  boolean cleanRun = false;
> >>  try {
> >>  runLoop();
> >>  cleanRun = true;
> >>  } catch (final Exception e) {
> >>  // we have caught all Kafka related exceptions, and other
> >> runtime exceptions
> >>  // should be due to user application errors
> >>
> >>  if (e instanceof UnsupportedVersionException) {
> >>  final String errorMessage = e.getMessage();
> >>  if (errorMessage != null &&
> >>  errorMessage.startsWith("Broker unexpectedly doesn't
> >> support requireStable flag on version ")) {
> >>
> >>  log.error("Shutting down because the Kafka cluster
> >> seems to be on a too old version. " +
> >>  "Setting {}=\"{}\" requires broker version 2.5 or
> >> higher.",
> >>  StreamsConfig.PROCESSING_GUARANTEE_CONFIG,
> >>  EXACTLY_ONCE_BETA);
> >>
> >>  throw e;
> >>  }
> >>  }
> >>
> >>  log.error("Encountered the following exception during
> processing " +
> >>  "and the thread is going to shut down: ", e);
> >>  throw e;
> >>  } finally {
> >>  completeShutdown(cleanRun);
> >>  }
> >> }
> >>
> >>
> >> Thanks and kind regards
> >>
> >>
> >> Scott Sinclair
>


Re: poll block question

2021-11-24 Thread Luke Chen
Hi xusheng,
I checked the code stack, and mapped to the kafka code, I can see we either
use Selector#selectNow, or Selector#select(timeoutMs), which should never
hang forever.
If so, I think it might be the `timeoutMS` is too large, and from your code
stack, the timeoutMS came from the Consumer#poll().
Could you reduce the poll timeout to see if that fixes the issue?

Also, I'm checking the code in v3.1, so it might also be possible that
there are some bugs in V2.3 and fixed in future release.

Thank you.
Luke

On Wed, Nov 24, 2021 at 11:12 PM xusheng  wrote:

> hi
> i find something not sure about poll.
> i need some help
> my kafka server version is 1.1.0
> my kafka client version is 2.3.0
> then 128 partition of topic xxx, start 128 thread in one application to
> consume message.
> always run well, but run about a week long , i find there is a consumer
> hang forever.
> strange is only one partition stop consume.
> i dump the Thread info :
>
> "ConsumerGroup-58" #302 prio=5 os_prio=0 tid=0x7f5419626800
> nid=0x191d9 runnable [0x7f540c95]
>
>java.lang.Thread.State: RUNNABLE
>
> at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>
> at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>
> at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>
> at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>
> - locked <0x0004461055c8> (a sun.nio.ch.Util$3)
>
> - locked <0x0004461055b8> (a java.util.Collections$UnmodifiableSet)
>
> - locked <0x000449932670> (a sun.nio.ch.EPollSelectorImpl)
>
> at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>
> at org.apache.kafka.common.network.Selector.select(Selector.java:794)
>
> at org.apache.kafka.common.network.Selector.poll(Selector.java:467)
>
> at org.apache.kafka.clients.NetworkClient.poll(NetworkClient.java:539)
>
> at
> org.apache.kafka.clients.consumer.internals.ConsumerNetworkClient.poll(ConsumerNetworkClient.java:262)
>
> at
> org.apache.kafka.clients.consumer.internals.ConsumerNetworkClient.poll(ConsumerNetworkClient.java:233)
>
> at
> org.apache.kafka.clients.consumer.KafkaConsumer.pollForFetches(KafkaConsumer.java:1281)
>
> at
> org.apache.kafka.clients.consumer.KafkaConsumer.poll(KafkaConsumer.java:1225)
>
> at
> org.apache.kafka.clients.consumer.KafkaConsumer.poll(KafkaConsumer.java:1201)
>
>
>
>
>
>


Re: KIP-800: Add reason to LeaveGroupRequest

2021-11-24 Thread Luke Chen
Hi David,
Sorry for the late reply.
Thanks for the update. It looks good now.
I love the idea to add joinGroup request reason, as well as the
`enforceRebalance` API!

One minor comment, since we extended the original KIP from leaveGroup to
joinGroup request, Could you please update the KIP title and also the
motivation section?
It makes the KIP much clearer for future reference.

Otherwise, LGTM!

Thank you.
Luke

On Fri, Nov 19, 2021 at 11:23 PM David Jacot 
wrote:

> I have updated the KIP.
>
> Best,
> David
>
> On Fri, Nov 19, 2021 at 3:00 PM David Jacot  wrote:
> >
> > Thank you all for your feedback. Let me address all your points below.
> >
> > Luke,
> > 1. I use a tag field so bumping the request version is not necessary. In
> > this case, using a tag field does not seem to be the best approach so
> > I will use a regular one and bump the version.
> > 2. Sounds good. I will fix that comment.
> > 3. That is a good question. My intent was to avoid getting weird or
> cryptic
> > reasons logged on the broker so I thought that having a standard one is
> > better. As Sophie suggested something similar for the `enforceRebalance`
> > API, we could do it for both, I suppose.
> >
> > Ismael,
> > 1. That's a good point. I chose to use a tag field to avoid having to
> bump
> > the request version. In this particular case, it seems that bumping the
> > version does not cost much so it is perhaps better. I will change this.
> >
> > Sophie,
> > 1. That's a pretty good idea, thanks. Let me update the KIP to include
> > a reason in the JoinGroup request. Regarding the Consumer API, do
> > you think that there is value for KStreams to expose the possibility to
> > provide a reason? Otherwise, it might be better to use a default
> > reason in this case.
> > 2. I don't fully get your point about providing the reason to the group
> > leader assignor on the client. Do you think that we should propagate
> > it to all the consumers or to the leader as well? The user usually has
> > access to all its client logs so I am not sure that it is really
> necessary.
> > Could you elaborate?
> >
> > I will update the KIP soon.
> >
> > Best,
> > David
> >
> > On Sat, Nov 13, 2021 at 6:00 AM Sophie Blee-Goldman
> >  wrote:
> > >
> > > This sounds great, thanks David.
> > >
> > > One thought: what do you think about doing something similar for the
> > > JoinGroup request?
> > >
> > > When you only have broker logs and not client logs, it's somewhere
> between
> > > challenging and
> > > impossible to determine the reason for a rebalance that was triggered
> > > explicitly by the client or
> > > even the user. For example, when a followup rebalance is requested to
> > > assign the revoked
> > > partitions after a cooperative rebalance. Or any of the many reasons we
> > > trigger a rebalance
> > > in Kafka Streams, via the #enforceRebalance API.
> > >
> > > Perhaps we could add a parameter to that method as such:
> > >
> > > public void enforceRebalance(final String reason);
> > >
> > > Then we can add that to the JoinGroup request/ConsumerProtocol. Not
> only
> > > would it help to
> > > log this reason on the broker side, the information about who
> requested the
> > > rebalance and why
> > > could be extremely useful to have available to the group
> leader/partition
> > > assignor on the client
> > > side.
> > >
> > > Cheers,
> > > Sophie
> > >
> > > On Fri, Nov 12, 2021 at 10:05 AM Ismael Juma 
> wrote:
> > >
> > > > Thanks David, this is a worthwhile improvement. Quick question, why
> did we
> > > > pick a tagged field here?
> > > >
> > > > Ismael
> > > >
> > > > On Thu, Nov 11, 2021, 8:32 AM David Jacot
> 
> > > > wrote:
> > > >
> > > > > Hi folks,
> > > > >
> > > > > I'd like to discuss this very small KIP which proposes to add a
> reason
> > > > > field
> > > > > to the LeaveGroupRequest in order to let the broker know why a
> member
> > > > > left the group. This would be really handy for administrators.
> > > > >
> > > > > KIP: https://cwiki.apache.org/confluence/x/eYyqCw
> > > > >
> > > > > Cheers,
> > > > > David
> > > > >
> > > >
>


Re: [DISCUSS] KIP-782: Expandable batch size in producer

2021-11-24 Thread Luke Chen
Hi Artem,
Yes, I agree if we go with random selection instead of round-robin
selection, the latency issue will be more fair. That is, if there are 10
partitions, the 10th partition will always be the last choice in each round
in current design, but with random selection, the chance to be selected is
more fair.

However, I think that's kind of out of scope with this KIP. This is an
existing issue, and it might need further discussion to decide if this
change is necessary.

I agree the default 32KB for "batch.max.size" might be not huge improvement
compared with 256KB. I'm thinking, maybe default to "64KB" for
"batch.max.size", and make the documentation clear that if the "batch.max.size"
is increased, there might be chances that the "ready" partitions need to
wait for next request to send to broker, because of the "max.request.size"
(default 1MB) limitation. "max.request.size" can also be considered to
increase to avoid this issue. What do you think?

Thank you.
Luke

On Wed, Nov 24, 2021 at 2:26 AM Artem Livshits
 wrote:

> >  maybe I can firstly decrease the "batch.max.size" to 32KB
>
> I think 32KB is too small.  With 5 in-flight and 100ms latency we can
> produce 1.6MB/s per partition.  With 256KB we can produce 12.8MB/s per
> partition.  We should probably set up some testing and see if 256KB has
> problems.
>
> To illustrate latency dynamics, let's consider a simplified model: 1
> in-flight request per broker, produce latency 125ms, 256KB max request
> size, 16 partitions assigned to the same broker, every second 128KB is
> produced to each partition (total production rate is 2MB/sec).
>
> If the batch size is 16KB, then the pattern would be the following:
>
> 0ms - produce 128KB into each partition
> 0ms - take 16KB from each partition send (total 256KB)
> 125ms - complete first 16KB from each partition, send next 16KB
> 250ms - complete second 16KB, send next 16KB
> ...
> 1000ms - complete 8th 16KB from each partition
>
> from this model it's easy to see that there are 256KB that are sent
> immediately, 256KB that are sent in 125ms, ... 256KB that are sent in
> 875ms.
>
> If the batch size is 256KB, then the pattern would be the following:
>
> 0ms - produce 128KB into each partition
> 0ms - take 128KB each from first 2 partitions and send (total 256KB)
> 125ms - complete 2 first partitions, send data from next 2 partitions
> ...
> 1000ms - complete last 2 partitions
>
> even though the pattern is different, there are still 256KB that are sent
> immediately, 256KB that are sent in 125ms, ... 256KB that are sent in
> 875ms.
>
> Now, in this example if we do strictly round-robin (current implementation)
> and we have this exact pattern (not sure how often such regular pattern
> would happen in practice -- I would expect that it would be a bit more
> random), some partitions would experience higher latency than others (not
> sure how much it would matter in practice -- in the end of the day some
> bytes produced to a topic would have higher latency and some bytes would
> have lower latency).  This pattern is easily fixed by choosing the next
> partition randomly instead of using round-robin.
>
> -Artem
>
> On Tue, Nov 23, 2021 at 12:08 AM Luke Chen  wrote:
>
> > Hi Tom,
> > Thanks for your comments. And thanks for Artem's explanation.
> > Below is my response:
> >
> > > Currently because buffers are allocated using batch.size it means we
> can
> > handle records that are that large (e.g. one big record per batch).
> Doesn't
> > the introduction of smaller buffer sizes (batch.initial.size) mean a
> > corresponding decrease in the maximum record size that the producer can
> > handle?
> >
> > Actually, the "batch.size" is only like a threshold to decide if the
> batch
> > is "ready to be sent". That is, even if you set the "batch.size=16KB"
> > (default value), users can still send one record sized with 20KB, as long
> > as the size is less than "max.request.size" in producer (default 1MB).
> > Therefore, the introduction of "batch.initial.size" won't decrease the
> > maximum record size that the producer can handle.
> >
> > > But isn't there the risk that drainBatchesForOneNode would end up not
> > sending ready
> > batches well past when they ought to be sent (according to their
> linger.ms
> > ),
> > because it's sending buffers for earlier partitions too aggressively?
> >
> > Did you mean that we have a "max.request.size" per request (default is
> > 1MB), and before this KIP, the request can include 64 batches in single
> > request ["batc

Re: [VOTE] KIP-798 Add possibility to write kafka headers in Kafka Console Producer

2021-11-23 Thread Luke Chen
Hi Florin,
I'm not a committer, but yes, this vote be concluded now.

Thank you.
Luke

On Wed, Nov 24, 2021 at 3:08 AM Florin Akermann 
wrote:

> Thanks all.
>
> The 72h window is through.
>
> @Comitters can this vote be concluded?
>
> The vote on KIP-798 would pass with:
> 4 binding +1
> 1 non-binding +1
> no vetoes
>
> Thanks,
> Florin
>
>
> On Tue, 23 Nov 2021 at 06:59, Luke Chen  wrote:
>
> > Hi Florin,
> > Thanks for the update!
> >
> > +1 (non-binding)
> >
> > Thank you.
> > Luke
> >
> > On Tue, Nov 23, 2021 at 2:00 AM Florin Akermann <
> florin.akerm...@gmail.com
> > >
> > wrote:
> >
> > > Hi Bill and David,
> > >
> > > Thank you both for the vote.
> > > @David: KIP is updated.
> > >
> > > Florin
> > >
> > > On Mon, 22 Nov 2021 at 18:28, David Jacot  >
> > > wrote:
> > >
> > > > Hi Florin,
> > > >
> > > > Thanks for the KIP. I am +1 (binding).
> > > >
> > > > There is a small typo in the Proposed Changes section:
> > > > `parse.header` should be `parse.headers`.
> > > >
> > > > Best,
> > > > David
> > > >
> > > > On Mon, Nov 22, 2021 at 6:20 PM Bill Bejeck 
> wrote:
> > > > >
> > > > > Hi Florin,
> > > > >
> > > > > Thanks for the KIP, this seems like a very useful addition.
> > > > >
> > > > > +1(binding).
> > > > >
> > > > > -Bill
> > > > >
> > > > > On Mon, Nov 22, 2021 at 12:00 PM Florin Akermann <
> > > > florin.akerm...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Luke and Tom
> > > > > >
> > > > > > @Tom: Thanks for the vote.
> > > > > >
> > > > > > @Luke: Thanks for the feedback.
> > > > > >
> > > > > > I have updated the KIP accordingly with regards to your comments
> on
> > > the
> > > > > > remaining case (false,false) and the motivation.
> > > > > >
> > > > > > Regarding the "not only UTF-8": As far as I understand John it is
> > > fine
> > > > to
> > > > > > limit the scope for this change to UTF-8 only as it is a handy
> > > > addition on
> > > > > > its own. Other formats can be relatively easily supported by
> adding
> > > > more
> > > > > > properties in later KIPs. In my reply to John (email from 21 Nov
> > > 2021,
> > > > > > 11:29 UTC) I also added an explanation why I limited the scope to
> > > UTF-8
> > > > > > only.
> > > > > >
> > > > > > Thanks,
> > > > > > Florin
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Mon, 22 Nov 2021 at 10:32, Tom Bentley 
> > > wrote:
> > > > > >
> > > > > > > Hi Florin,
> > > > > > >
> > > > > > > Thanks for the KIP!
> > > > > > >
> > > > > > > +1 (binding),
> > > > > > >
> > > > > > > Kind regards,
> > > > > > >
> > > > > > > Tom
> > > > > > >
> > > > > > > On Mon, Nov 22, 2021 at 6:51 AM Luke Chen 
> > > wrote:
> > > > > > >
> > > > > > > > Hi Florin,
> > > > > > > > Thanks for the KIP.
> > > > > > > >
> > > > > > > > This KIP makes sense to me. Just a comment that the
> motivation
> > > > section
> > > > > > is
> > > > > > > > not clearly explain why this KIP is important.
> > > > > > > > I think John already mentioned a good motivation, which is to
> > > > support
> > > > > > > "not
> > > > > > > > only UTF-8".
> > > > > > > > You should put that into the KIP, and of course if you have
> > other
> > > > > > > thoughts,
> > > > > > > > please also add them into KIP.
> > > > > > > >
> > > > > > > > Also, in the "public interface" section, there are 3 "

Re: [DISCUSS] KIP-782: Expandable batch size in producer

2021-11-23 Thread Luke Chen
r
> > batch.initial.size < batch.size it could cause regressions for existing
> > users with a large record size, I think. It should be enough for
> > batch.initial.size to default to batch.size, allowing users who care
> about
> > the memory saving in the off-peak throughput case to do the tuning, but
> not
> > causing a regression for existing users.
> >
> > I think this KIP would change the behaviour of producers when there are
> > multiple partitions ready to be sent: By sending all the ready buffers
> > (which may now be > batch.size) for the first partition, we could end up
> > excluding ready buffers for other partitions from the current send. In
> > other words, as I understand the KIP currently, there's a change in
> > fairness. I think the code in RecordAccumulator#drainBatchesForOneNode
> will
> > ensure fairness in the long run, because the drainIndex will ensure that
> > those other partitions each get their turn at being the first. But isn't
> > there the risk that drainBatchesForOneNode would end up not sending ready
> > batches well past when they ought to be sent (according to their
> linger.ms
> > ),
> > because it's sending buffers for earlier partitions too aggressively? Or,
> > to put it another way, perhaps the RecordAccumulator should round-robin
> the
> > ready buffers for _all_ the partitions before trying to fill the
> remaining
> > space with the extra buffers (beyond the batch.size limit) for the first
> > partitions?
> >
> > Kind regards,
> >
> > Tom
> >
> > On Wed, Oct 20, 2021 at 1:35 PM Luke Chen  wrote:
> >
> > > Hi Ismael and all devs,
> > > Is there any comments/suggestions to this KIP?
> > > If no, I'm going to update the KIP based on my previous mail, and
> start a
> > > vote tomorrow or next week.
> > >
> > > Thank you.
> > > Luke
> > >
> > > On Mon, Oct 18, 2021 at 2:40 PM Luke Chen  wrote:
> > >
> > > > Hi Ismael,
> > > > Thanks for your comments.
> > > >
> > > > 1. Why do we have to reallocate the buffer? We can keep a list of
> > buffers
> > > > instead and avoid reallocation.
> > > > -> Do you mean we allocate multiple buffers with
> "buffer.initial.size",
> > > > and link them together (with linked list)?
> > > > ex:
> > > > a. We allocate 4KB initial buffer
> > > > | 4KB |
> > > >
> > > > b. when new records reached and the remaining buffer is not enough
> for
> > > the
> > > > records, we create another batch with "batch.initial.size" buffer
> > > > ex: we already have 3KB of data in the 1st buffer, and here comes the
> > 2KB
> > > > record
> > > >
> > > > | 4KB (1KB remaining) |
> > > > now, record: 2KB coming
> > > > We fill the 1st 1KB into 1st buffer, and create new buffer, and
> linked
> > > > together, and fill the rest of data into it
> > > > | 4KB (full) | ---> | 4KB (3KB remaining) |
> > > >
> > > > Is that what you mean?
> > > > If so, I think I like this idea!
> > > > If not, please explain more detail about it.
> > > > Thank you.
> > > >
> > > > 2. I think we should also consider tweaking the semantics of
> batch.size
> > > so
> > > > that the sent batches can be larger if the batch is not ready to be
> > sent
> > > > (while still respecting max.request.size and perhaps a new
> > > max.batch.size).
> > > >
> > > > --> In the KIP, I was trying to make the "batch.size" as the upper
> > bound
> > > > of the batch size, and introduce a "batch.initial.size" as initial
> > batch
> > > > size.
> > > > So are you saying that we can let "batch.size" as initial batch size
> > and
> > > > introduce a "max.batch.size" as upper bound value?
> > > > That's a good suggestion, but that would change the semantics of
> > > > "batch.size", which might surprise some users. I think my original
> > > proposal
> > > > ("batch.initial.size") is safer for users. What do you think?
> > > >
> > > > Thank you.
> > > > Luke
> > > >
> > > >
> > > > On Mon, Oct 18, 2021 at 3:12 AM Ismael Juma 
> wrote:
> > > >
> > > >> I think we should also consider tweaking the semanti

Re: [VOTE] KIP-798 Add possibility to write kafka headers in Kafka Console Producer

2021-11-22 Thread Luke Chen
Hi Florin,
Thanks for the update!

+1 (non-binding)

Thank you.
Luke

On Tue, Nov 23, 2021 at 2:00 AM Florin Akermann 
wrote:

> Hi Bill and David,
>
> Thank you both for the vote.
> @David: KIP is updated.
>
> Florin
>
> On Mon, 22 Nov 2021 at 18:28, David Jacot 
> wrote:
>
> > Hi Florin,
> >
> > Thanks for the KIP. I am +1 (binding).
> >
> > There is a small typo in the Proposed Changes section:
> > `parse.header` should be `parse.headers`.
> >
> > Best,
> > David
> >
> > On Mon, Nov 22, 2021 at 6:20 PM Bill Bejeck  wrote:
> > >
> > > Hi Florin,
> > >
> > > Thanks for the KIP, this seems like a very useful addition.
> > >
> > > +1(binding).
> > >
> > > -Bill
> > >
> > > On Mon, Nov 22, 2021 at 12:00 PM Florin Akermann <
> > florin.akerm...@gmail.com>
> > > wrote:
> > >
> > > > Hi Luke and Tom
> > > >
> > > > @Tom: Thanks for the vote.
> > > >
> > > > @Luke: Thanks for the feedback.
> > > >
> > > > I have updated the KIP accordingly with regards to your comments on
> the
> > > > remaining case (false,false) and the motivation.
> > > >
> > > > Regarding the "not only UTF-8": As far as I understand John it is
> fine
> > to
> > > > limit the scope for this change to UTF-8 only as it is a handy
> > addition on
> > > > its own. Other formats can be relatively easily supported by adding
> > more
> > > > properties in later KIPs. In my reply to John (email from 21 Nov
> 2021,
> > > > 11:29 UTC) I also added an explanation why I limited the scope to
> UTF-8
> > > > only.
> > > >
> > > > Thanks,
> > > > Florin
> > > >
> > > >
> > > >
> > > > On Mon, 22 Nov 2021 at 10:32, Tom Bentley 
> wrote:
> > > >
> > > > > Hi Florin,
> > > > >
> > > > > Thanks for the KIP!
> > > > >
> > > > > +1 (binding),
> > > > >
> > > > > Kind regards,
> > > > >
> > > > > Tom
> > > > >
> > > > > On Mon, Nov 22, 2021 at 6:51 AM Luke Chen 
> wrote:
> > > > >
> > > > > > Hi Florin,
> > > > > > Thanks for the KIP.
> > > > > >
> > > > > > This KIP makes sense to me. Just a comment that the motivation
> > section
> > > > is
> > > > > > not clearly explain why this KIP is important.
> > > > > > I think John already mentioned a good motivation, which is to
> > support
> > > > > "not
> > > > > > only UTF-8".
> > > > > > You should put that into the KIP, and of course if you have other
> > > > > thoughts,
> > > > > > please also add them into KIP.
> > > > > >
> > > > > > Also, in the "public interface" section, there are 3 "Default
> > parsing
> > > > > > pattern", I think you should add 1 remaining case (false, false)
> to
> > > > make
> > > > > it
> > > > > > complete.
> > > > > >
> > > > > > Otherwise, look good to me.
> > > > > >
> > > > > > Thank you.
> > > > > > Luke
> > > > > >
> > > > > >
> > > > > > On Sun, Nov 21, 2021 at 7:37 PM Florin Akermann <
> > > > > florin.akerm...@gmail.com
> > > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi John,
> > > > > > >
> > > > > > > Thanks for the vote and feedback.
> > > > > > >
> > > > > > > The thought occurred to me too.
> > > > > > >
> > > > > > > Do I understand it correctly: the current version of the
> > > > > > > kafka-console-producer cannot be used for anything other than
> > UTF-8
> > > > > keys
> > > > > > > and values?
> > > > > > > (There is no other implementation of MessageReader other than
> the
> > > > > > > ConsoleProducer$LineMessageReader)
> > > > > > > In other words, currently users seem to only apply it with
> utf-8
> > > > > strings
> > > > > > > for keys and values?
> > > > > > > This is why I figured I would not deviate from this assumption
> > solely
> > > > > for
> > > > > > > the headers.
> > > > > > >
> > > > > > > I will happily raise another KIP / Jira if there is a need to
> > specify
> > > > > > other
> > > > > > > formats / serializers for headers, keys and/or values.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Florin
> > > > > > >
> > > > > > >
> > > > > > > On Sat, 20 Nov 2021 at 19:34, John Roesler <
> vvcep...@apache.org>
> > > > > wrote:
> > > > > > >
> > > > > > > > Hi Florin,
> > > > > > > >
> > > > > > > > Thanks for the KIP!
> > > > > > > >
> > > > > > > > I think the assumption that header values are UTF-8 strings
> > might
> > > > not
> > > > > > > hold
> > > > > > > > up in the long run, but it seems like we can easily add a
> > property
> > > > > > later
> > > > > > > to
> > > > > > > > specify the format. It seems like this scope is probably a
> > handy
> > > > > > addition
> > > > > > > > on its own.
> > > > > > > >
> > > > > > > > I’m +1 (binding)
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > John
> > > > > > > >
> > > > > > > >
> > > > > > > > On Fri, Nov 19, 2021, at 15:06, flo wrote:
> > > > > > > > > <
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-798%3A+Add+possibility+to+write+kafka+headers+in+Kafka+Console+Producer
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
>


Re: [VOTE] KIP-797: Accept duplicate listener on port for IPv4/IPv6

2021-11-22 Thread Luke Chen
Hi Matthew,
Thanks for the KIP.
It makes sense to allow IPv4 and IPv6 listening on the same port for the
listener config.

+1 (non-binding)

Thank you.
Luke

On Mon, Nov 22, 2021 at 6:28 PM Matthew de Detrich
 wrote:

> Hello everyone,
>
> I would like to start a vote for KIP-797: Accept duplicate listener on port
> for IPv4/IPv6
>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=195726330
>
> The purpose of the KIP is to loosen current validation for non advertised
> listeners so that you can have an IPv4 address and an IPv6 address on the
> same port. All other behaviour remains the same as before (since these are
> disparate IP stacks there are no technical reasons not to allow this).
>
> PR is located at https://github.com/apache/kafka/pull/11478
>
> Comments and feedback are welcome!
>
> Regards
>
> --
>
> Matthew de Detrich
>
> *Aiven Deutschland GmbH*
>
> Immanuelkirchstraße 26, 10405 Berlin
>
> Amtsgericht Charlottenburg, HRB 209739 B
>
> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
>
> *m:* +491603708037
>
> *w:* aiven.io *e:* matthew.dedetr...@aiven.io
>


Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-22 Thread Luke Chen
Hello David,

For (3):



* I suppose that we could add a `generation` field to the JoinGroupRequest
instead to do the fencing that you describe while handling the sentinel in
the assignor directly. If we would add the `generation` to the request
itself, would we need the `generation` in the subscription protocol as
well?*

On second thought, I think this is not better than adding `generation`
field in the subscription protocol, because I think we don't have to do any
generation validation on joinGroup request. The purpose of
`joinGroupRequest` is to accept any members to join this group, even if the
member is new or ever joined or what. As long as we have the generationId
in the subscription metadata, the consumer lead can leverage the info to
ignore the old ownedPartitions (or do other handling), and the rebalance
can still complete successfully and correctly. On the other hand, if we did
the generation check on JoinGroupRequest, and return `ILLEGAL_GENERATION`
back to consumer, the consumer needs to clear its generation info and
rejoin the group to continue the rebalance. It needs more request/response
network and slow down the rebalance.

So I think we should add the `generationId` field into Subscription
protocol to achieve what we want.

Thank you.
Luke

On Thu, Nov 18, 2021 at 8:51 PM Luke Chen  wrote:

> Hi David,
> Thanks for your feedback.
>
> I've updated the KIP for your comments (1)(2).
> For (3), it's a good point! Yes, we didn't deserialize the subscription
> metadata on broker side, and it's not necessary to add overhead on broker
> side. And, yes, I think we can fix the original issue by adding a
> "generation" field into `JoinGroupRequest` instead, and also add a field
> into `JoinGroupResponse` in `JoinGroupResponseMember` field. That way, the
> broker can identify the old member from `JoinGroupRequest`. And the
> assignor can also get the "generation" info via the `Subscription` instance.
>
> I'll update the KIP to add "generation" field into `JoinGroupRequest` and
> `JoinGroupResponse`, if there is no other options.
>
> Thank you.
> Luke
>
>
> On Tue, Nov 16, 2021 at 12:31 AM David Jacot 
> wrote:
>
>> Hi Luke,
>>
>> Thanks for the KIP. Overall, I think that the motivation makes sense. I
>> have a couple of comments/questions:
>>
>> 1. In the Public Interfaces section, it would be great if you could put
>> the
>> end state not the current one.
>>
>> 2. Do we need to update the Subscription class to expose the
>> generation? If so, it would be great to mention it in the Public
>> Interfaces section as well.
>>
>> 3. You mention that the broker will set the generation if the subscription
>> contains a sentinel value (-1). As of today, the broker does not parse
>> the subscription so I am not sure how/why we would do this. I suppose
>> that we could add a `generation` field to the JoinGroupRequest instead
>> to do the fencing that you describe while handling the sentinel in the
>> assignor directly. If we would add the `generation` to the request itself,
>> would we need the `generation` in the subscription protocol as well?
>>
>> Best,
>> David
>>
>> On Fri, Nov 12, 2021 at 3:31 AM Luke Chen  wrote:
>> >
>> > Hi all,
>> >
>> > I'd like to start the discussion for KIP-792: Add "generation" field
>> into
>> > consumer protocol.
>> >
>> > The goal of this KIP is to allow assignor/consumer coordinator/group
>> > coordinator to have a way to identify the out-of-date
>> members/assignments.
>> >
>> > Detailed description can be found here:
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614
>> >
>> > Any feedback is welcome.
>> >
>> > Thank you.
>> > Luke
>>
>


Re: [RESULTS] [VOTE] Release Kafka version 2.7.2

2021-11-21 Thread Luke Chen
Hi Felixzh,
> So, Is version-2.7.2 ready now?
Not yet.
If it is released, you'll see 2.7.2 appeared in the download page here:
https://kafka.apache.org/downloads

Thank you.
Luke

On Mon, Nov 22, 2021 at 3:05 PM felixzh  wrote:

> So, Is version-2.7.2 ready now?
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> At 2021-11-12 17:43:23, "Mickael Maison"  wrote:
> >This vote passes with 6 +1 votes (3 bindings) and no 0 or -1 votes.
> >
> >+1 votes
> >PMC Members:
> >* Bill Bejeck
> >* Manikumar Reddy
> >* Konstantine Karantasis
> >
> >Committers:
> >* Tom Bentley
> >
> >Community:
> >* Israel Ekpo
> >* Luke Chen
> >
> >0 votes
> >* No votes
> >
> >-1 votes
> >* No votes
> >
> >Vote thread:
> >https://lists.apache.org/thread/whzns1rj8ythxtfyz18hog1m3y6l215d
> >
> >I'll continue with the release process and the release announcement
> >will follow in the next few days.
> >
> >Mickael
>


Re: [VOTE] KIP-798 Add possibility to write kafka headers in Kafka Console Producer

2021-11-21 Thread Luke Chen
Hi Florin,
Thanks for the KIP.

This KIP makes sense to me. Just a comment that the motivation section is
not clearly explain why this KIP is important.
I think John already mentioned a good motivation, which is to support "not
only UTF-8".
You should put that into the KIP, and of course if you have other thoughts,
please also add them into KIP.

Also, in the "public interface" section, there are 3 "Default parsing
pattern", I think you should add 1 remaining case (false, false) to make it
complete.

Otherwise, look good to me.

Thank you.
Luke


On Sun, Nov 21, 2021 at 7:37 PM Florin Akermann 
wrote:

> Hi John,
>
> Thanks for the vote and feedback.
>
> The thought occurred to me too.
>
> Do I understand it correctly: the current version of the
> kafka-console-producer cannot be used for anything other than UTF-8 keys
> and values?
> (There is no other implementation of MessageReader other than the
> ConsoleProducer$LineMessageReader)
> In other words, currently users seem to only apply it with utf-8 strings
> for keys and values?
> This is why I figured I would not deviate from this assumption solely for
> the headers.
>
> I will happily raise another KIP / Jira if there is a need to specify other
> formats / serializers for headers, keys and/or values.
>
> Thanks,
> Florin
>
>
> On Sat, 20 Nov 2021 at 19:34, John Roesler  wrote:
>
> > Hi Florin,
> >
> > Thanks for the KIP!
> >
> > I think the assumption that header values are UTF-8 strings might not
> hold
> > up in the long run, but it seems like we can easily add a property later
> to
> > specify the format. It seems like this scope is probably a handy addition
> > on its own.
> >
> > I’m +1 (binding)
> >
> > Thanks,
> > John
> >
> >
> > On Fri, Nov 19, 2021, at 15:06, flo wrote:
> > > <
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-798%3A+Add+possibility+to+write+kafka+headers+in+Kafka+Console+Producer
> > >
> >
>


Re: [ANNOUNCE] New Kafka PMC Member: Tom Bentley

2021-11-18 Thread Luke Chen
Congrats, Tom!

Guozhang Wang  於 2021年11月19日 週五 上午1:13 寫道:

> Congrats Tom!
>
> Guozhang
>
> On Thu, Nov 18, 2021 at 7:49 AM Jun Rao  wrote:
>
> > Hi, Everyone,
> >
> > Tom Bentley has been a Kafka committer since Mar. 15,  2021. He has been
> > very instrumental to the community since becoming a committer. It's my
> > pleasure to announce that Tom is now a member of Kafka PMC.
> >
> > Congratulations Tom!
> >
> > Jun
> > on behalf of Apache Kafka PMC
> >
>
>
> --
> -- Guozhang
>


Re: [DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-18 Thread Luke Chen
Hi David,
Thanks for your feedback.

I've updated the KIP for your comments (1)(2).
For (3), it's a good point! Yes, we didn't deserialize the subscription
metadata on broker side, and it's not necessary to add overhead on broker
side. And, yes, I think we can fix the original issue by adding a
"generation" field into `JoinGroupRequest` instead, and also add a field
into `JoinGroupResponse` in `JoinGroupResponseMember` field. That way, the
broker can identify the old member from `JoinGroupRequest`. And the
assignor can also get the "generation" info via the `Subscription` instance.

I'll update the KIP to add "generation" field into `JoinGroupRequest` and
`JoinGroupResponse`, if there is no other options.

Thank you.
Luke


On Tue, Nov 16, 2021 at 12:31 AM David Jacot 
wrote:

> Hi Luke,
>
> Thanks for the KIP. Overall, I think that the motivation makes sense. I
> have a couple of comments/questions:
>
> 1. In the Public Interfaces section, it would be great if you could put the
> end state not the current one.
>
> 2. Do we need to update the Subscription class to expose the
> generation? If so, it would be great to mention it in the Public
> Interfaces section as well.
>
> 3. You mention that the broker will set the generation if the subscription
> contains a sentinel value (-1). As of today, the broker does not parse
> the subscription so I am not sure how/why we would do this. I suppose
> that we could add a `generation` field to the JoinGroupRequest instead
> to do the fencing that you describe while handling the sentinel in the
> assignor directly. If we would add the `generation` to the request itself,
> would we need the `generation` in the subscription protocol as well?
>
> Best,
> David
>
> On Fri, Nov 12, 2021 at 3:31 AM Luke Chen  wrote:
> >
> > Hi all,
> >
> > I'd like to start the discussion for KIP-792: Add "generation" field into
> > consumer protocol.
> >
> > The goal of this KIP is to allow assignor/consumer coordinator/group
> > coordinator to have a way to identify the out-of-date
> members/assignments.
> >
> > Detailed description can be found here:
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614
> >
> > Any feedback is welcome.
> >
> > Thank you.
> > Luke
>


Re: [DISCUSS] KIP-797 Accept duplicate listener on port for IPv4/IPv6

2021-11-18 Thread Luke Chen
Hi Matthew,
Thanks for the update.
The "listener" config description should also be updated:
https://kafka.apache.org/documentation/#brokerconfigs_listeners

We stated: Listener names and port numbers must be unique.
It'll be wrong after this KIP.

Also, the PR link should not be put onto the KIP. You can put in the JIRA
comment.

Thank you.
Luke

On Thu, Nov 18, 2021 at 7:34 PM Matthew de Detrich
 wrote:

> Hi Luke,
>
> I have updated the KIP to make it clear that we are only talking about
> listeners and not advertised listeners. I have also updated/rebased the PR
> at https://github.com/apache/kafka/pull/11478 to add upgrade notes about
> the suggested change. I am not sure if there is an additional place where
> you want me to document this change (I looked through the documentation and
> couldn't find anything specific enough but I may have missed something).
>
> Let me know if anything else is needed.
>
> Regards
>
> On Thu, Nov 18, 2021 at 3:58 AM Luke Chen  wrote:
>
> > Hi Matthew,
> > Thanks for the KIP.
> >
> > I have a question:
> > If I remembered correctly, the "advertised listeners" already support
> > duplicated ports, so your KIP should only focus on "listeners"
> > configuration, is that right? If so, could you please make it clear in
> KIP,
> > to mention that your change only apply to "listeners", not "advertised
> > listeners".
> >
> > Also, you should also mention in the KIP, that the doc for "listeners"
> will
> > also be updated. (I checked your PR, and found you missed that)
> >
> > Thank you.
> > Luke
> >
> > On Tue, Nov 16, 2021 at 10:24 PM Matthew de Detrich
> >  wrote:
> >
> > > Since no one has commented on either this thread or the original one I
> > will
> > > summon a vote by the end of this week.
> > >
> > > Regards
> > >
> > > On Wed, Nov 10, 2021 at 5:28 PM Matthew de Detrich <
> > > matthew.dedetr...@aiven.io> wrote:
> > >
> > > > Hello everyone,
> > > >
> > > > I would like to start a discussion for KIP-797 which is about
> allowing
> > > > duplicate listeners on the same port in the specific case where one
> > host
> > > is
> > > > an IPv4 address and the other host is an IPv6 address.
> > > >
> > > > The proposal is here
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=195726330
> > > >
> > > > Regards
> > > > --
> > > >
> > > > Matthew de Detrich
> > > >
> > > > *Aiven Deutschland GmbH*
> > > >
> > > > Immanuelkirchstraße 26, 10405 Berlin
> > > >
> > > > Amtsgericht Charlottenburg, HRB 209739 B
> > > >
> > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> > > >
> > > > *m:* +491603708037
> > > >
> > > > *w:* aiven.io *e:* matthew.dedetr...@aiven.io
> > > >
> > >
> > >
> > > --
> > >
> > > Matthew de Detrich
> > >
> > > *Aiven Deutschland GmbH*
> > >
> > > Immanuelkirchstraße 26, 10405 Berlin
> > >
> > > Amtsgericht Charlottenburg, HRB 209739 B
> > >
> > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> > >
> > > *m:* +491603708037
> > >
> > > *w:* aiven.io *e:* matthew.dedetr...@aiven.io
> > >
> >
>
>
> --
>
> Matthew de Detrich
>
> *Aiven Deutschland GmbH*
>
> Immanuelkirchstraße 26, 10405 Berlin
>
> Amtsgericht Charlottenburg, HRB 209739 B
>
> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
>
> *m:* +491603708037
>
> *w:* aiven.io *e:* matthew.dedetr...@aiven.io
>


Re: [DISCUSS] KIP-797 Accept duplicate listener on port for IPv4/IPv6

2021-11-17 Thread Luke Chen
Hi Matthew,
Thanks for the KIP.

I have a question:
If I remembered correctly, the "advertised listeners" already support
duplicated ports, so your KIP should only focus on "listeners"
configuration, is that right? If so, could you please make it clear in KIP,
to mention that your change only apply to "listeners", not "advertised
listeners".

Also, you should also mention in the KIP, that the doc for "listeners" will
also be updated. (I checked your PR, and found you missed that)

Thank you.
Luke

On Tue, Nov 16, 2021 at 10:24 PM Matthew de Detrich
 wrote:

> Since no one has commented on either this thread or the original one I will
> summon a vote by the end of this week.
>
> Regards
>
> On Wed, Nov 10, 2021 at 5:28 PM Matthew de Detrich <
> matthew.dedetr...@aiven.io> wrote:
>
> > Hello everyone,
> >
> > I would like to start a discussion for KIP-797 which is about allowing
> > duplicate listeners on the same port in the specific case where one host
> is
> > an IPv4 address and the other host is an IPv6 address.
> >
> > The proposal is here
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=195726330
> >
> > Regards
> > --
> >
> > Matthew de Detrich
> >
> > *Aiven Deutschland GmbH*
> >
> > Immanuelkirchstraße 26, 10405 Berlin
> >
> > Amtsgericht Charlottenburg, HRB 209739 B
> >
> > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> >
> > *m:* +491603708037
> >
> > *w:* aiven.io *e:* matthew.dedetr...@aiven.io
> >
>
>
> --
>
> Matthew de Detrich
>
> *Aiven Deutschland GmbH*
>
> Immanuelkirchstraße 26, 10405 Berlin
>
> Amtsgericht Charlottenburg, HRB 209739 B
>
> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
>
> *m:* +491603708037
>
> *w:* aiven.io *e:* matthew.dedetr...@aiven.io
>


Re: [DISCUSS] Brokers disconnect intermittently with TLS1.3

2021-11-15 Thread Luke Chen
Hi Shylaja,
Thanks for reporting the issue.
> Given that TLS1.3 does not support renegotiation, can I make it
applicable just for TLS1.2?
Are you saying you're trying to make Kafka default supports to TLS1.2,
instead of TLS1.3?
If so, I don't think it's a good idea to fall back to an older and weaker
security protocol just because of a bug.
Instead, I think we should try to investigate it and fix it from the root.

So, are you sure this is a issue that `renegotiation` is not supported by
TLSv1.3?
Could we fix it?

Thank you.
Luke

On Tue, Nov 16, 2021 at 4:05 AM Kokoori, Shylaja 
wrote:

> Hi all,
>
> Using TLS1.3 (with JDK11) is causing an intermittent increase in
> inter-broker p99 latency, as mentioned by Yiming in Kafka-9320<
> https://issues.apache.org/jira/browse/KAFKA-9320?focusedCommentId=17401818=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17401818>.
> We tested this with Kafka 2.8.
> The issue seems to be because of a renegotiation exception being thrown by
>
> read(ByteBuffer dst)
>
> &
>
> write(ByteBuffer src)
>
> in
>
> clients/src/main/java/org/apache/kafka/common/network/SslTransportLayer.java
>
> This exception is causing the connection to close between the brokers
> before read/write is completed.
>
> In our internal experiments we have seen the p99 latency stabilize when we
> remove this exception.
>
> Given that TLS1.3 does not support renegotiation, can I make it applicable
> just for TLS1.2?
>
> I have also created a ticket<
> https://issues.apache.org/jira/browse/KAFKA-13418>
>
> Any feedback is welcome.
>
> Thank you,
>
> Shylaja
>
>
>
>


Re: KIP-800: Add reason to LeaveGroupRequest

2021-11-11 Thread Luke Chen
Hi David,
Thanks for the KIP.
It makes sense to me.

Some comments:
1. Should we bump the `LeaveGroupRequest` protocol version to 5?
2. the description in the new field: "about": "The reason" -> "about": "The
reason why the member left the group"
3. For the `removeMembersFromConsumerGroup` case, do you think we can add a
new "reason" field in `RemoveMembersFromConsumerGroupOptions`, and default
to "the consumer was removed by an admin." as you proposed? I think when
getting the reason "removed by an admin" is not quite helpful for
troubleshooting, right? So, with the new field, it allows users to be able
to customize the reason if necessary.  for troubleshooting, What do you
think?

Thank you.
Luke

On Fri, Nov 12, 2021 at 12:32 AM David Jacot 
wrote:

> Hi folks,
>
> I'd like to discuss this very small KIP which proposes to add a reason
> field
> to the LeaveGroupRequest in order to let the broker know why a member
> left the group. This would be really handy for administrators.
>
> KIP: https://cwiki.apache.org/confluence/x/eYyqCw
>
> Cheers,
> David
>


[DISCUSS] KIP-792: Add "generation" field into consumer protocol

2021-11-11 Thread Luke Chen
Hi all,

I'd like to start the discussion for KIP-792: Add "generation" field into
consumer protocol.

The goal of this KIP is to allow assignor/consumer coordinator/group
coordinator to have a way to identify the out-of-date members/assignments.

Detailed description can be found here:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=191336614

Any feedback is welcome.

Thank you.
Luke


Re: [VOTE] KIP-782: Expandable batch size in producer

2021-11-09 Thread Luke Chen
Hi devs,
Bump this thread.
Call for vote for: KIP-782: Expandable batch size in producer.

The main goal for this KIP is:
1. higher throughput in producer
2. better memory usage in producer

Detailed description can be found here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer

Any feedback and comments is welcome.

Thank you.
Luke

On Fri, Nov 5, 2021 at 4:37 PM Luke Chen  wrote:

> Hi Mickael,
> Thanks for the good comments! Answering them below:
>
> - When under load, the producer may allocate extra buffers. Are these
> buffers ever released if the load drops?
> --> This is a good point that I've never considered before. Yes, after
> introducing the "batch.max.size", we should release some buffer out of the
> buffer pools. In this KIP, we'll only keep maximum "batch.size" into pool,
> and mark the rest of memory as free to use. The reason we keep maximum
> "batch.size" back to pool is because the semantic of "batch.size" is the
> batch full limit. In most cases, the batch.size should be able to contain
> the records to be sent within linger.ms time.
>
> - Do we really need batch.initial.size? It's not clear that having this
> extra setting adds a lot of value.
> --> I think "batch.initial.size" is important to achieve higher memory
> usage. Now, I made the default value to 4KB, so after upgrading to the new
> release, the producer memory usage will become better.
>
> I've updated the KIP.
>
> Thank you.
> Luke
>
> On Wed, Nov 3, 2021 at 6:44 PM Mickael Maison 
> wrote:
>
>> Hi Luke,
>>
>> Thanks for the KIP. It looks like an interesting idea. I like the
>> concept of dynamically adjusting settings to handle load. I wonder if
>> other client settings could also benefit from a similar logic.
>>
>> Just a couple of questions:
>> - When under load, the producer may allocate extra buffers. Are these
>> buffers ever released if the load drops?
>> - Do we really need batch.initial.size? It's not clear that having
>> this extra setting adds a lot of value.
>>
>> Thanks,
>> Mickael
>>
>> On Tue, Oct 26, 2021 at 11:12 AM Luke Chen  wrote:
>> >
>> > Thank you, Artem!
>> >
>> > @devs, welcome to vote for this KIP.
>> > Key proposal:
>> > 1. allocate multiple smaller initial batch size buffer in producer, and
>> > list them together when expansion for better memory usage
>> > 2. add a max batch size config in producer, so when producer rate is
>> > suddenly high, we can still have high throughput with batch size larger
>> > than "batch.size" (and less than "batch.max.size", where "batch.size" is
>> > soft limit and "batch.max.size" is hard limit)
>> > Here's the updated KIP:
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
>> >
>> > And, any comments and feedback are welcome.
>> >
>> > Thank you.
>> > Luke
>> >
>> > On Tue, Oct 26, 2021 at 6:35 AM Artem Livshits
>> >  wrote:
>> >
>> > > Hi Luke,
>> > >
>> > > I've looked at the updated KIP-782, it looks good to me.
>> > >
>> > > -Artem
>> > >
>> > > On Sun, Oct 24, 2021 at 1:46 AM Luke Chen  wrote:
>> > >
>> > > > Hi Artem,
>> > > > Thanks for your good suggestion again.
>> > > > I've combined your idea into this KIP, and updated it.
>> > > > Note, in the end, I still keep the "batch.initial.size" config
>> (default
>> > > is
>> > > > 0, which means "batch.size" will be initial batch size) for better
>> memory
>> > > > conservation.
>> > > >
>> > > > Detailed description can be found here:
>> > > >
>> > >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
>> > > >
>> > > > Let me know if you have other suggestions.
>> > > >
>> > > > Thank you.
>> > > > Luke
>> > > >
>> > > > On Sat, Oct 23, 2021 at 10:50 AM Luke Chen 
>> wrote:
>> > > >
>> > > >> Hi Artem,
>> > > >> Thanks for the suggestion. Let me confirm my understanding is
>> correct.
>> > > >> So, what you suggest is that the "batch.size" is more like a "soft
>> > > limit"
>> > > &

Re: [VOTE] KIP-791: Add Record Metadata to State Store Context

2021-11-08 Thread Luke Chen
Hi Patrick,
Thanks for the KIP.
Adding RecordMetadata into StateStoreContext for offset updating makes
sense to me.

+1 (non-binding)

Thank you.
Luke


On Mon, Nov 8, 2021 at 5:18 PM Patrick Stuedi 
wrote:

> Hi all,
>
> Thanks for the feedback on KIP-791, I have updated the KIP and would like
> to start the voting.
>
> The KIP can be found here:
> https://cwiki.apache.org/confluence/x/I5BnCw
>
> Please vote in this thread.
>
> Thanks!
> -Patrick
>


Re: [DISCUSS] KIP-794: Strictly Uniform Sticky Partitioner

2021-11-08 Thread Luke Chen
Thanks Artem,
It's much better now.
I've got your idea. In KIP-480: Sticky Partitioner, we'll change partition
(call partitioner) when either 1 of below condition match
1. the batch is full
2. when linger.ms is up
But, you are changing the definition, into a
"partitioner.sticky.batch.size" size is reached.

It'll fix the uneven distribution issue, because we did the sent out size
calculation in the producer side.
But it might have another issue that when the producer rate is low, there
will be some period of time the distribution is not even. Ex:
tp-1: 12KB
tp-2: 0KB
tp-3: 0KB
tp-4: 0KB
while the producer is still keeping sending records into tp-1 (because we
haven't reached the 16KB threshold)
Maybe the user should set a good value to "partitioner.sticky.batch.size"
to fix this issue?

Some comment to the KIP:
1. This paragraph is a little confusing, because there's no "batch mode" or
"non-batch mode", right?

> The batching will continue until either an in-flight batch completes or
we hit the N bytes and move to the next partition.  This way it takes just
5 records to get to batching mode, not 5 x number of partition records, and
the batching mode will stay longer as we'll be batching while waiting for a
request to be completed.

Even with linger.ms=0, before the sender thread is ready, we're always
batching (accumulating) records into batches. So I think the "batch mode"
description is confusing. And that's why I asked you if you have some kind
of "batch switch" here.

2. In motivation, you mentioned 1 drawback of current
UniformStickyPartitioner is "the sticky partitioner doesn't create batches
as efficiently", because it sent out a batch with only 1 record (under
linger.ms=0). But I can't tell how you fix this un-efficient issue in the
proposed solution. I still see we sent 1 record within 1 batch. Could you
explain more here?

Thank you.
Luke

On Sat, Nov 6, 2021 at 6:41 AM Artem Livshits
 wrote:

> Hi Luke,
>
> Thank you for your feedback.  I've updated the KIP with your suggestions.
>
> 1. Updated with a better example.
> 2. I removed the reference to ClassicDefaultPartitioner, it was probably
> confusing.
> 3. The logic doesn't rely on checking batches, I've updated the proposal to
> make it more explicit.
> 4. The primary issue (uneven distribution) is described in the linked jira,
> copied an example from jira into the KIP as well.
>
> -Artem
>
>
> On Thu, Nov 4, 2021 at 8:34 PM Luke Chen  wrote:
>
> > Hi Artem,
> > Thanks for the KIP! And thanks for reminding me to complete KIP-782,
> soon.
> > :)
> >
> > Back to the KIP, I have some comments:
> > 1. You proposed to have a new config: "partitioner.sticky.batch.size",
> but
> > I can't see how we're going to use it to make the partitioner better.
> > Please explain more in KIP (with an example will be better as suggestion
> > (4))
> > 2. In the "Proposed change" section, you take an example to use
> > "ClassicDefaultPartitioner", is that referring to the current default
> > sticky partitioner? I think it'd better you name your proposed partition
> > with a different name for distinguish between the default one and new
> one.
> > (Although after implementation, we are going to just use the same name)
> > 3. So, if my understanding is correct, you're going to have a "batch"
> > switch, and before the in-flight is full, it's disabled. Otherwise, we'll
> > enable it. Is that right? Sorry, I don't see any advantage of having this
> > batch switch. Could you explain more?
> > 4. I think it should be more clear if you can have a clear real example
> in
> > the motivation section, to describe what issue we faced using current
> > sticky partitioner. And in proposed changes section, using the same
> > example, to describe more detail about how you fix this issue with your
> > way.
> >
> > Thank you.
> > Luke
> >
> > On Fri, Nov 5, 2021 at 1:38 AM Artem Livshits
> >  wrote:
> >
> > > Hello,
> > >
> > > This is the discussion thread for
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-794%3A+Strictly+Uniform+Sticky+Partitioner
> > > .
> > >
> > > The proposal is a bug fix for
> > > https://issues.apache.org/jira/browse/KAFKA-10888, but it does
> include a
> > > client config change, therefore we have a KIP to discuss.
> > >
> > > -Artem
> > >
> >
>


Re: [VOTE] KIP-782: Expandable batch size in producer

2021-11-05 Thread Luke Chen
Hi Mickael,
Thanks for the good comments! Answering them below:

- When under load, the producer may allocate extra buffers. Are these
buffers ever released if the load drops?
--> This is a good point that I've never considered before. Yes, after
introducing the "batch.max.size", we should release some buffer out of the
buffer pools. In this KIP, we'll only keep maximum "batch.size" into pool,
and mark the rest of memory as free to use. The reason we keep maximum
"batch.size" back to pool is because the semantic of "batch.size" is the
batch full limit. In most cases, the batch.size should be able to contain
the records to be sent within linger.ms time.

- Do we really need batch.initial.size? It's not clear that having this
extra setting adds a lot of value.
--> I think "batch.initial.size" is important to achieve higher memory
usage. Now, I made the default value to 4KB, so after upgrading to the new
release, the producer memory usage will become better.

I've updated the KIP.

Thank you.
Luke

On Wed, Nov 3, 2021 at 6:44 PM Mickael Maison 
wrote:

> Hi Luke,
>
> Thanks for the KIP. It looks like an interesting idea. I like the
> concept of dynamically adjusting settings to handle load. I wonder if
> other client settings could also benefit from a similar logic.
>
> Just a couple of questions:
> - When under load, the producer may allocate extra buffers. Are these
> buffers ever released if the load drops?
> - Do we really need batch.initial.size? It's not clear that having
> this extra setting adds a lot of value.
>
> Thanks,
> Mickael
>
> On Tue, Oct 26, 2021 at 11:12 AM Luke Chen  wrote:
> >
> > Thank you, Artem!
> >
> > @devs, welcome to vote for this KIP.
> > Key proposal:
> > 1. allocate multiple smaller initial batch size buffer in producer, and
> > list them together when expansion for better memory usage
> > 2. add a max batch size config in producer, so when producer rate is
> > suddenly high, we can still have high throughput with batch size larger
> > than "batch.size" (and less than "batch.max.size", where "batch.size" is
> > soft limit and "batch.max.size" is hard limit)
> > Here's the updated KIP:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
> >
> > And, any comments and feedback are welcome.
> >
> > Thank you.
> > Luke
> >
> > On Tue, Oct 26, 2021 at 6:35 AM Artem Livshits
> >  wrote:
> >
> > > Hi Luke,
> > >
> > > I've looked at the updated KIP-782, it looks good to me.
> > >
> > > -Artem
> > >
> > > On Sun, Oct 24, 2021 at 1:46 AM Luke Chen  wrote:
> > >
> > > > Hi Artem,
> > > > Thanks for your good suggestion again.
> > > > I've combined your idea into this KIP, and updated it.
> > > > Note, in the end, I still keep the "batch.initial.size" config
> (default
> > > is
> > > > 0, which means "batch.size" will be initial batch size) for better
> memory
> > > > conservation.
> > > >
> > > > Detailed description can be found here:
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
> > > >
> > > > Let me know if you have other suggestions.
> > > >
> > > > Thank you.
> > > > Luke
> > > >
> > > > On Sat, Oct 23, 2021 at 10:50 AM Luke Chen 
> wrote:
> > > >
> > > >> Hi Artem,
> > > >> Thanks for the suggestion. Let me confirm my understanding is
> correct.
> > > >> So, what you suggest is that the "batch.size" is more like a "soft
> > > limit"
> > > >> batch size, and the "hard limit" is "batch.max.size". When reaching
> the
> > > >> batch.size of the buffer, it means the buffer is "ready" to be be
> sent.
> > > But
> > > >> before the linger.ms reached, if there are more data coming, we can
> > > >> still accumulate it into the same buffer, until it reached the
> > > >> "batch.max.size". After it reached the "batch.max.size", we'll
> create
> > > >> another batch for it.
> > > >>
> > > >> So after your suggestion, we won't need the "batch.initial.size",
> and we
> > > >> can use "batch.size" as the initial batch size. We list each
> > > "batch.size"
> > > 

Re: [DISCUSS] KIP-794: Strictly Uniform Sticky Partitioner

2021-11-04 Thread Luke Chen
Hi Artem,
Thanks for the KIP! And thanks for reminding me to complete KIP-782, soon.
:)

Back to the KIP, I have some comments:
1. You proposed to have a new config: "partitioner.sticky.batch.size", but
I can't see how we're going to use it to make the partitioner better.
Please explain more in KIP (with an example will be better as suggestion
(4))
2. In the "Proposed change" section, you take an example to use
"ClassicDefaultPartitioner", is that referring to the current default
sticky partitioner? I think it'd better you name your proposed partition
with a different name for distinguish between the default one and new one.
(Although after implementation, we are going to just use the same name)
3. So, if my understanding is correct, you're going to have a "batch"
switch, and before the in-flight is full, it's disabled. Otherwise, we'll
enable it. Is that right? Sorry, I don't see any advantage of having this
batch switch. Could you explain more?
4. I think it should be more clear if you can have a clear real example in
the motivation section, to describe what issue we faced using current
sticky partitioner. And in proposed changes section, using the same
example, to describe more detail about how you fix this issue with your way.

Thank you.
Luke

On Fri, Nov 5, 2021 at 1:38 AM Artem Livshits
 wrote:

> Hello,
>
> This is the discussion thread for
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-794%3A+Strictly+Uniform+Sticky+Partitioner
> .
>
> The proposal is a bug fix for
> https://issues.apache.org/jira/browse/KAFKA-10888, but it does include a
> client config change, therefore we have a KIP to discuss.
>
> -Artem
>


Re: [DISCUSS] KIP-786: Emit Metric Client Quota Values

2021-11-04 Thread Luke Chen
Hi Mason,
Thanks for the KIP.
But I think since the quota value won't change from time to time unless
admin alter it, it might be waste of resources to record it on each
produce/fetch API.
It can alternatively be achieved by using the kafka-configs.sh to describe
ALL users/clients/default to have a overview of the quota values when
needed.

What do you think?

Thank you.
Luke,


On Wed, Nov 3, 2021 at 10:53 PM Mickael Maison 
wrote:

> Hi Mason,
>
> Thanks for the KIP. I think it's a good idea to also emit quota limits
> as metrics. It certainly simplifies monitoring/graphing if all the
> data come from the same source.
>
> The KIP looks good overall, just a couple of questions:
> - Have you considered enabling the new metrics by default?
> - If you prefer keeping a configuration to enable them, what about
> renaming it to "client.quota.value.metric.enable" or even
> "quota.value.metric.enable"?
>
> Thanks,
> Mickael
>
> On Wed, Oct 27, 2021 at 11:36 PM Mason Legere
>  wrote:
> >
> > Hi All,
> >
> > Haven't received any feedback on this yet but as it was a small change
> have
> > made a PR showing the functional components: pull request
> > 
> > Will update the related documentation outlining the new metric attributes
> > in a bit.
> >
> > Best,
> > Mason Legere
> >
> > On Sat, Oct 23, 2021 at 4:00 PM Mason Legere <
> mason.leg...@salesforce.com>
> > wrote:
> >
> > > Hi All,
> > >
> > > I would like to start a discussion for my proposed KIP-786
> > > <
> https://cwiki.apache.org/confluence/pages/resumedraft.action?draftId=191335406=9a2f3d65-5633-47c8-994c-f5a14738cb1e;>
> which
> > > aims to allow client quota values to be emitted as a standard jmx MBean
> > > attribute - if enabled in the static broker configuration.
> > >
> > > Please note that I originally misnumbered this KIP and am re-creating
> this
> > > discussion thread for clarity. The original thread can be found at:
> Original
> > > Email Thread
> > > <
> https://lists.apache.org/thread.html/r44e154761f22a42e4766f2098d1e33cb54865311f41648ebd9406a4f%40%3Cdev.kafka.apache.org%3E
> >
> > >
> > > Best,
> > > Mason Legere
> > >
>


Re: [VOTE] KIP-788: Allow configuring num.network.threads per listener

2021-11-04 Thread Luke Chen
Hi Mickael,
Thanks for the KIP.
It's great to have the capability to fine tune the number of threads per
listener!

Just 2 minor comments for the KIP:
1. The discussion thread is not attached in KIP
2. Israel raised the case-sensitive comment and your response didn't put
into the KIP

Otherwise, LGTM!
+1 (non-binding)

Thank you.
Luke

On Wed, Nov 3, 2021 at 8:17 PM Mickael Maison 
wrote:

> Hi all,
>
> I'd like to start the vote on KIP-788. It will allow setting the
> number of network threads per listener.
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-788%3A+Allow+configuring+num.network.threads+per+listener
>
> Please let me know if you have any feedback.
> Thanks
>


[jira] [Created] (KAFKA-13420) consumer protocol should include "generation" field for assignor to distinguish between new/old OwnedPartitions

2021-10-29 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13420:
-

 Summary: consumer protocol should include "generation" field for 
assignor to distinguish between new/old OwnedPartitions
 Key: KAFKA-13420
 URL: https://issues.apache.org/jira/browse/KAFKA-13420
 Project: Kafka
  Issue Type: Improvement
  Components: clients, consumer
Reporter: Luke Chen
Assignee: Luke Chen


In 
[KIP-429|https://cwiki.apache.org/confluence/display/KAFKA/KIP-429%3A+Kafka+Consumer+Incremental+Rebalance+Protocol],
 we add a new field: `OwnedPartitions` into consumer protocol, for cooperative 
protocol do partition revoking things. But recently, we found the 
`ownedPartitions` info might be out-of-date due to some reasons (ex: unstable 
network), and the out-of-date  `ownedPartitions` causes unexpected rebalance 
stuck issue (ex: KAFKA-12984, KAFKA-13406). To fix it, we should consider to 
add the "generation" field in the consumer protocol, so that we can rely on the 
"generation" info to identify if the `ownedPartition` is up-to-date or 
out-of-date.



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


[jira] [Created] (KAFKA-13419) sync group failed with retriable error might cause out-of-date ownedPartition in Cooperative protocol

2021-10-29 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13419:
-

 Summary: sync group failed with retriable error might cause 
out-of-date ownedPartition in Cooperative protocol
 Key: KAFKA-13419
 URL: https://issues.apache.org/jira/browse/KAFKA-13419
 Project: Kafka
  Issue Type: Bug
  Components: clients
Affects Versions: 3.0.0
Reporter: Luke Chen
Assignee: Luke Chen


In KAFKA-13406, we found there's user got stuck when in rebalancing with 
cooperative sticky assignor. The reason is the "ownedPartition" is out-of-date, 
and it failed the cooperative assignment validation.

Investigate deeper, I found the root cause is we didn't reset generation and 
state after sync group fail. In KAFKA-12983, we fixed the issue that the 
onJoinPrepare is not called in resetStateAndRejoin method. And it causes the 
ownedPartition not get cleared. But there's another case that the 
ownedPartition will be out-of-date. Here's the example:
 # consumer A joined and synced group successfully with generation 1
 # New rebalance started with generation 2, consumer A joined successfully, but 
somehow, consumer A doesn't send out sync group immediately
 # other consumer completed sync group successfully in generation 2, except 
consumer A.
 # After consumer A send out sync group, the new rebalance start, with 
generation 3. So consumer A got REBALANCE_IN_PROGRESS error with sync group 
response
 # When receiving REBALANCE_IN_PROGRESS, we re-join the group, with generation 
3, with the assignment (ownedPartition) in generation 1.
 # So, now, we have out-of-date ownedPartition sent, with unexpected results 
happened

 

We might want to do resetStateAndRejoin when retriable errors happend in *sync 
group*. Because when we got sync group error, it means, join group passed, and 
other consumers (and the leader) might already completed this round of 
rebalance. The assignment distribution this consumer have is already 
out-of-date.

 

The errors should resetStateAndRejoin in sync group are:
{code:java}
if (exception instanceof UnknownMemberIdException ||
exception instanceof IllegalGenerationException ||
exception instanceof RebalanceInProgressException ||
exception instanceof MemberIdRequiredException)
continue;
else if (!future.isRetriable())
throw exception;
{code}



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


Re: New Kafka Consumer : unknown member id

2021-10-28 Thread Luke Chen
Hi,
Which version of kafka client are you using?
I can't find this error message in the source code.
When googling this error message, it showed the error is in Kafka v0.9.

Could you try to use the V3.0.0 and see if that issue still exist?

Thank you.
Luke

On Thu, Oct 28, 2021 at 11:15 PM Kafka Life  wrote:

> Dear Kafka Experts
>
> We have set up a group.id (consumer ) = YYY
> But when tried to connect to kafka instance : i get this error message. I
> am sure this consumer (group id does not exist in kafka) .We user plain
> text protocol to connect to kafka 2.8.0. Please suggest how to resolve this
> issue.
>
> DEBUG AbstractCoordinator:557 - [Consumer clientId=X, groupId=YYY]
> Attempt to join group failed due to unknown member id.
>


Re: [VOTE] KIP-784: Add top-level error code field to DescribeLogDirsResponse

2021-10-27 Thread Luke Chen
Hi Mickael,
Thanks for the KIP.
It's good to keep it consistent with others, to have top-level error field.

+ 1 (non-binding)

Thank you.
Luke

On Wed, Oct 27, 2021 at 9:01 PM Mickael Maison 
wrote:

> Hi all,
>
> I'd like to start the vote on this minor KIP.
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-784%3A+Add+top-level+error+code+field+to+DescribeLogDirsResponse
>
> Please take a look, vote or let me know if you have any feedback.
>
> Thanks
>


Re: [VOTE] KIP-780: Support fine-grained compression options

2021-10-27 Thread Luke Chen
Hi Dongjin,
Thanks for the KIP.
+1 (non-binding)

Luke

On Wed, Oct 27, 2021 at 8:44 PM Dongjin Lee  wrote:

> Bumping up the voting thread.
>
> If you have any questions or opinions, don't hesitate to leave them in the
> discussion thread.
>
> Best,
> Dongjin
>
> On Thu, Oct 14, 2021 at 3:02 AM Dongjin Lee  wrote:
>
> > Hi, Kafka dev,
> >
> > I'd like to open a vote for KIP-780: Support fine-grained compression
> > options:
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-780%3A+Support+fine-grained+compression+options
> >
> > Please note that this feature mutually complements KIP-390: Support
> > Compression Level (accepted, targeted to 3.1.0.). It was initially
> planned
> > for a part of KIP-390 but spun off for performance concerns.
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-390%3A+Support+Compression+Level
> >
> > Best,
> > Dongjin
> >
> > --
> > *Dongjin Lee*
> >
> > *A hitchhiker in the mathematical world.*
> >
> >
> >
> > *github:  github.com/dongjinleekr
> > keybase:
> https://keybase.io/dongjinleekr
> > linkedin:
> kr.linkedin.com/in/dongjinleekr
> > speakerdeck:
> speakerdeck.com/dongjin
> > *
> >
>
>
> --
> *Dongjin Lee*
>
> *A hitchhiker in the mathematical world.*
>
>
>
> *github:  github.com/dongjinleekr
> keybase: https://keybase.io/dongjinleekr
> linkedin: kr.linkedin.com/in/dongjinleekr
> speakerdeck:
> speakerdeck.com/dongjin
> *
>


[jira] [Created] (KAFKA-13406) Cooperative sticky assignor got stuck due to assignment validation failed

2021-10-27 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13406:
-

 Summary: Cooperative sticky assignor got stuck due to assignment 
validation failed
 Key: KAFKA-13406
 URL: https://issues.apache.org/jira/browse/KAFKA-13406
 Project: Kafka
  Issue Type: Bug
  Components: clients
Affects Versions: 3.0.0
Reporter: Luke Chen
Assignee: Luke Chen


We'll do validateCooperativeAssignment for cooperative assignor, where we 
validate if there are previously owned partitions directly transfer to other 
consumers without "revoke" step. However, the "ownedPartition" in subscription 
might contain out-of-dated data, which might cause the validation always 
failure.

We should consider the fix it by deserializing the subscription userData for 
generation info in validateCooperationAssignment.



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


Re: [VOTE] KIP-782: Expandable batch size in producer

2021-10-26 Thread Luke Chen
Thank you, Artem!

@devs, welcome to vote for this KIP.
Key proposal:
1. allocate multiple smaller initial batch size buffer in producer, and
list them together when expansion for better memory usage
2. add a max batch size config in producer, so when producer rate is
suddenly high, we can still have high throughput with batch size larger
than "batch.size" (and less than "batch.max.size", where "batch.size" is
soft limit and "batch.max.size" is hard limit)
Here's the updated KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer

And, any comments and feedback are welcome.

Thank you.
Luke

On Tue, Oct 26, 2021 at 6:35 AM Artem Livshits
 wrote:

> Hi Luke,
>
> I've looked at the updated KIP-782, it looks good to me.
>
> -Artem
>
> On Sun, Oct 24, 2021 at 1:46 AM Luke Chen  wrote:
>
> > Hi Artem,
> > Thanks for your good suggestion again.
> > I've combined your idea into this KIP, and updated it.
> > Note, in the end, I still keep the "batch.initial.size" config (default
> is
> > 0, which means "batch.size" will be initial batch size) for better memory
> > conservation.
> >
> > Detailed description can be found here:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
> >
> > Let me know if you have other suggestions.
> >
> > Thank you.
> > Luke
> >
> > On Sat, Oct 23, 2021 at 10:50 AM Luke Chen  wrote:
> >
> >> Hi Artem,
> >> Thanks for the suggestion. Let me confirm my understanding is correct.
> >> So, what you suggest is that the "batch.size" is more like a "soft
> limit"
> >> batch size, and the "hard limit" is "batch.max.size". When reaching the
> >> batch.size of the buffer, it means the buffer is "ready" to be be sent.
> But
> >> before the linger.ms reached, if there are more data coming, we can
> >> still accumulate it into the same buffer, until it reached the
> >> "batch.max.size". After it reached the "batch.max.size", we'll create
> >> another batch for it.
> >>
> >> So after your suggestion, we won't need the "batch.initial.size", and we
> >> can use "batch.size" as the initial batch size. We list each
> "batch.size"
> >> together, until it reached "batch.max.size". Something like this:
> >>
> >> [image: image.png]
> >> Is my understanding correct?
> >> If so, that sounds good to me.
> >> If not, please kindly explain more to me.
> >>
> >> Thank you.
> >> Luke
> >>
> >>
> >>
> >>
> >> On Sat, Oct 23, 2021 at 2:13 AM Artem Livshits
> >>  wrote:
> >>
> >>> Hi Luke,
> >>>
> >>> Nice suggestion.  It should optimize how memory is used with different
> >>> production rates, but I wonder if we can take this idea further and
> >>> improve
> >>> batching in general.
> >>>
> >>> Currently batch.size is used in two conditions:
> >>>
> >>> 1. When we append records to a batch in the accumulator, we create a
> new
> >>> batch if the current batch would exceed the batch.size.
> >>> 2. When we drain the batch from the accumulator, a batch becomes
> 'ready'
> >>> when it reaches batch.size.
> >>>
> >>> The second condition is good with the current batch size, because if
> >>> linger.ms is greater than 0, the send can be triggered by
> accomplishing
> >>> the
> >>> batching goal.
> >>>
> >>> The first condition, though, leads to creating many batches if the
> >>> network
> >>> latency or production rate (or both) is high, and with 5 in-flight and
> >>> 16KB
> >>> batches we can only have 80KB of data in-flight per partition.  Which
> >>> means
> >>> that with 50ms latency, we can only push ~1.6MB/sec per partition (this
> >>> goes down if we consider higher latencies, e.g. with 100ms we can only
> >>> push
> >>> ~0.8MB/sec).
> >>>
> >>> I think it would be great to separate the two sizes:
> >>>
> >>> 1. When appending records to a batch, create a new batch if the current
> >>> exceeds a larger size (we can call it batch.max.size), say 256KB by
> >>> default.
> >>> 2. When we drain, consider batch 'ready' if it exceeds batch.size,

Re: [kafka-clients] [VOTE] 2.7.2 RC0

2021-10-26 Thread Luke Chen
Hi Mickael,

Thanks for the release. I did:
1. Verified checksums and signatures
2. Run quick start steps
3. Verified the CVE-2021-38153 is indeed fixed in kafka-2.7.2-src.tgz
.

+1 (non-binding)

Thank you.
Luke

On Tue, Oct 26, 2021 at 3:41 PM Tom Bentley  wrote:

> Hi Mickael,
>
> As with 2.6.3 RC0, I have:
>
> * Verified checksums and signatures
> * Built jars and docs from the source jar
> * Run the unit and integration tests
>
> +1 non-binding
>
> Kind regards,
>
> Tom
>
> On Sun, Oct 24, 2021 at 3:05 PM Israel Ekpo  wrote:
>
> > Mickael,
> >
> > Do we need to do another RC? Were there issues with this release?
> >
> > What happens next?
> >
> >
> > On Sat, Oct 16, 2021 at 8:11 PM Israel Ekpo 
> wrote:
> >
> > >
> > > I have performed the following checks
> > >
> > > Validation of Release Artifacts Cryptographic Hashes (ASC MD5 SHA1
> > SHA512)
> > > PGP Signatures used to sign the release artifacts
> > > Javadocs check
> > > Site docs check was not necessary
> > > Jenkins build was successful.
> > >
> > > I used the steps here for the first two checks
> > > https://github.com/izzyacademy/apache-kafka-release-party
> > >
> > > I vote +1 on this RC
> > >
> > >
> > > On Fri, Oct 15, 2021 at 12:11 PM Israel Ekpo 
> > wrote:
> > >
> > >> Hi Mickael,
> > >>
> > >> I am pretty surprised that there are no votes so far on the RCs and
> the
> > >> deadline has already passed.
> > >>
> > >> I am running my checks right now using the process outlined here
> > >>
> > >>
> > >>
> >
> https://github.com/izzyacademy/apache-kafka-release-party#how-to-validate-apache-kafka-release-candidates
> > >>
> > >> I will post my results and vote as soon as they are completed.
> > >>
> > >> On Fri, Oct 15, 2021 at 9:52 AM Mickael Maison 
> > >> wrote:
> > >>
> > >>> Successful Jenkins build:
> > >>> https://ci-builds.apache.org/job/Kafka/job/kafka-2.7-jdk8/181/
> > >>>
> > >>> On Wed, Oct 13, 2021 at 6:47 PM Mickael Maison 
> > >>> wrote:
> > >>> >
> > >>> > Hi Israel,
> > >>> >
> > >>> > Our tooling generates the same template for all types of releases.
> > >>> >
> > >>> > For bugfix releases, the site docs and javadocs don't typically
> > >>> > require extensive validation.
> > >>> > It's still a good idea to open them up and check a few pages to
> > >>> > validate they look right.
> > >>> >
> > >>> > For this release, as you've mentioned, site docs have not changed.
> > >>> >
> > >>> > Thanks
> > >>> >
> > >>> > On Wed, Oct 13, 2021 at 1:59 AM Israel Ekpo 
> > >>> wrote:
> > >>> > >
> > >>> > > Mickael,
> > >>> > >
> > >>> > > For patch or bug fix releases like this one, should we exclude
> the
> > >>> Javadocs and site docs if they have not changed?
> > >>> > >
> > >>> > > https://github.com/apache/kafka-site
> > >>> > >
> > >>> > > The site docs were last changed about 6 months ago and it appears
> > it
> > >>> may not have changed or needs validation
> > >>> > >
> > >>> > >
> > >>> > >
> > >>> > > On Tue, Oct 12, 2021 at 2:17 PM Mickael Maison <
> > mimai...@apache.org>
> > >>> wrote:
> > >>> > >>
> > >>> > >> Hello Kafka users, developers and client-developers,
> > >>> > >>
> > >>> > >> This is the first candidate for release of Apache Kafka 2.7.2.
> > >>> > >>
> > >>> > >> Apache Kafka 2.7.2 is a bugfix release and 26 issues, as well as
> > >>> > >> CVE-2021-38153, have been fixed since 2.7.1.
> > >>> > >>
> > >>> > >> Release notes for the 2.7.2 release:
> > >>> > >>
> > >>> https://home.apache.org/~mimaison/kafka-2.7.2-rc0/RELEASE_NOTES.html
> > >>> > >>
> > >>> > >> *** Please download, test and vote by Friday, October 15, 5pm
> CET
> > >>> > >>
> > >>> > >> Kafka's KEYS file containing PGP keys we use to sign the
> release:
> > >>> > >> https://kafka.apache.org/KEYS
> > >>> > >>
> > >>> > >> * Release artifacts to be voted upon (source and binary):
> > >>> > >> https://home.apache.org/~mimaison/kafka-2.7.2-rc0/
> > >>> > >>
> > >>> > >> * Maven artifacts to be voted upon:
> > >>> > >>
> > >>>
> https://repository.apache.org/content/groups/staging/org/apache/kafka/
> > >>> > >>
> > >>> > >> * Javadoc:
> > >>> > >> https://home.apache.org/~mimaison/kafka-2.7.2-rc0/javadoc/
> > >>> > >>
> > >>> > >> * Tag to be voted upon (off 2.7 branch) is the 2.7.2 tag:
> > >>> > >> https://github.com/apache/kafka/releases/tag/2.7.2-rc0
> > >>> > >>
> > >>> > >> * Documentation:
> > >>> > >> https://kafka.apache.org/27/documentation.html
> > >>> > >>
> > >>> > >> * Protocol:
> > >>> > >> https://kafka.apache.org/27/protocol.html
> > >>> > >>
> > >>> > >> * Successful Jenkins builds for the 2.7 branch:
> > >>> > >> I'll share a link once the build completes
> > >>> > >>
> > >>> > >>
> > >>> > >> /**
> > >>> > >>
> > >>> > >> Thanks,
> > >>> > >> Mickael
> > >>> > >>
> > >>> > >> --
> > >>> > >> You received this message because you are subscribed to the
> Google
> > >>> Groups "kafka-clients" group.
> > >>> > >> To unsubscribe 

Re: [VOTE] KIP-782: Expandable batch size in producer

2021-10-24 Thread Luke Chen
Hi Artem,
Thanks for your good suggestion again.
I've combined your idea into this KIP, and updated it.
Note, in the end, I still keep the "batch.initial.size" config (default is
0, which means "batch.size" will be initial batch size) for better memory
conservation.

Detailed description can be found here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer

Let me know if you have other suggestions.

Thank you.
Luke

On Sat, Oct 23, 2021 at 10:50 AM Luke Chen  wrote:

> Hi Artem,
> Thanks for the suggestion. Let me confirm my understanding is correct.
> So, what you suggest is that the "batch.size" is more like a "soft limit"
> batch size, and the "hard limit" is "batch.max.size". When reaching the
> batch.size of the buffer, it means the buffer is "ready" to be be sent. But
> before the linger.ms reached, if there are more data coming, we can still
> accumulate it into the same buffer, until it reached the "batch.max.size".
> After it reached the "batch.max.size", we'll create another batch for it.
>
> So after your suggestion, we won't need the "batch.initial.size", and we
> can use "batch.size" as the initial batch size. We list each "batch.size"
> together, until it reached "batch.max.size". Something like this:
>
> [image: image.png]
> Is my understanding correct?
> If so, that sounds good to me.
> If not, please kindly explain more to me.
>
> Thank you.
> Luke
>
>
>
>
> On Sat, Oct 23, 2021 at 2:13 AM Artem Livshits
>  wrote:
>
>> Hi Luke,
>>
>> Nice suggestion.  It should optimize how memory is used with different
>> production rates, but I wonder if we can take this idea further and
>> improve
>> batching in general.
>>
>> Currently batch.size is used in two conditions:
>>
>> 1. When we append records to a batch in the accumulator, we create a new
>> batch if the current batch would exceed the batch.size.
>> 2. When we drain the batch from the accumulator, a batch becomes 'ready'
>> when it reaches batch.size.
>>
>> The second condition is good with the current batch size, because if
>> linger.ms is greater than 0, the send can be triggered by accomplishing
>> the
>> batching goal.
>>
>> The first condition, though, leads to creating many batches if the network
>> latency or production rate (or both) is high, and with 5 in-flight and
>> 16KB
>> batches we can only have 80KB of data in-flight per partition.  Which
>> means
>> that with 50ms latency, we can only push ~1.6MB/sec per partition (this
>> goes down if we consider higher latencies, e.g. with 100ms we can only
>> push
>> ~0.8MB/sec).
>>
>> I think it would be great to separate the two sizes:
>>
>> 1. When appending records to a batch, create a new batch if the current
>> exceeds a larger size (we can call it batch.max.size), say 256KB by
>> default.
>> 2. When we drain, consider batch 'ready' if it exceeds batch.size, which
>> is
>> 16KB by default.
>>
>> For memory conservation we may introduce batch.initial.size if we want to
>> have a flexibility to make it even smaller than batch.size, or we can just
>> always use batch.size as the initial size (in which case we don't
>> need batch.initial.size config).
>>
>> -Artem
>>
>> On Fri, Oct 22, 2021 at 1:52 AM Luke Chen  wrote:
>>
>> > Hi Kafka dev,
>> > I'd like to start a vote for the proposal: KIP-782: Expandable batch
>> size
>> > in producer.
>> >
>> > The main purpose for this KIP is to have better memory usage in
>> producer,
>> > and also save users from the dilemma while setting the batch size
>> > configuration. After this KIP, users can set a higher batch.size without
>> > worries, and of course, with an appropriate "batch.initial.size".
>> >
>> > Derailed description can be found here:
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
>> >
>> > Any comments and feedback are welcome.
>> >
>> > Thank you.
>> > Luke
>> >
>>
>


[jira] [Created] (KAFKA-13396) kafka-topics.sh --create should not require the partitions/replication-factor argument

2021-10-23 Thread Luke Chen (Jira)
Luke Chen created KAFKA-13396:
-

 Summary: kafka-topics.sh --create should not require the 
partitions/replication-factor argument
 Key: KAFKA-13396
 URL: https://issues.apache.org/jira/browse/KAFKA-13396
 Project: Kafka
  Issue Type: Bug
Affects Versions: 3.0.0
Reporter: Luke Chen
Assignee: Luke Chen


When removing the support of --zookeeper for kafka-topics.sh in KAFKA-12596, we 
accidentally add a constraint in --create case, that the 
partitions/replication-factor are now required. This constraint should only 
apply for zookeeper case.



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


Re: [VOTE] KIP-782: Expandable batch size in producer

2021-10-22 Thread Luke Chen
Hi Artem,
Thanks for the suggestion. Let me confirm my understanding is correct.
So, what you suggest is that the "batch.size" is more like a "soft limit"
batch size, and the "hard limit" is "batch.max.size". When reaching the
batch.size of the buffer, it means the buffer is "ready" to be be sent. But
before the linger.ms reached, if there are more data coming, we can still
accumulate it into the same buffer, until it reached the "batch.max.size".
After it reached the "batch.max.size", we'll create another batch for it.

So after your suggestion, we won't need the "batch.initial.size", and we
can use "batch.size" as the initial batch size. We list each "batch.size"
together, until it reached "batch.max.size". Something like this:

[image: image.png]
Is my understanding correct?
If so, that sounds good to me.
If not, please kindly explain more to me.

Thank you.
Luke




On Sat, Oct 23, 2021 at 2:13 AM Artem Livshits
 wrote:

> Hi Luke,
>
> Nice suggestion.  It should optimize how memory is used with different
> production rates, but I wonder if we can take this idea further and improve
> batching in general.
>
> Currently batch.size is used in two conditions:
>
> 1. When we append records to a batch in the accumulator, we create a new
> batch if the current batch would exceed the batch.size.
> 2. When we drain the batch from the accumulator, a batch becomes 'ready'
> when it reaches batch.size.
>
> The second condition is good with the current batch size, because if
> linger.ms is greater than 0, the send can be triggered by accomplishing
> the
> batching goal.
>
> The first condition, though, leads to creating many batches if the network
> latency or production rate (or both) is high, and with 5 in-flight and 16KB
> batches we can only have 80KB of data in-flight per partition.  Which means
> that with 50ms latency, we can only push ~1.6MB/sec per partition (this
> goes down if we consider higher latencies, e.g. with 100ms we can only push
> ~0.8MB/sec).
>
> I think it would be great to separate the two sizes:
>
> 1. When appending records to a batch, create a new batch if the current
> exceeds a larger size (we can call it batch.max.size), say 256KB by
> default.
> 2. When we drain, consider batch 'ready' if it exceeds batch.size, which is
> 16KB by default.
>
> For memory conservation we may introduce batch.initial.size if we want to
> have a flexibility to make it even smaller than batch.size, or we can just
> always use batch.size as the initial size (in which case we don't
> need batch.initial.size config).
>
> -Artem
>
> On Fri, Oct 22, 2021 at 1:52 AM Luke Chen  wrote:
>
> > Hi Kafka dev,
> > I'd like to start a vote for the proposal: KIP-782: Expandable batch size
> > in producer.
> >
> > The main purpose for this KIP is to have better memory usage in producer,
> > and also save users from the dilemma while setting the batch size
> > configuration. After this KIP, users can set a higher batch.size without
> > worries, and of course, with an appropriate "batch.initial.size".
> >
> > Derailed description can be found here:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
> >
> > Any comments and feedback are welcome.
> >
> > Thank you.
> > Luke
> >
>


[VOTE] KIP-782: Expandable batch size in producer

2021-10-22 Thread Luke Chen
Hi Kafka dev,
I'd like to start a vote for the proposal: KIP-782: Expandable batch size
in producer.

The main purpose for this KIP is to have better memory usage in producer,
and also save users from the dilemma while setting the batch size
configuration. After this KIP, users can set a higher batch.size without
worries, and of course, with an appropriate "batch.initial.size".

Derailed description can be found here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer

Any comments and feedback are welcome.

Thank you.
Luke


Re: [DISCUSS] KIP-782: Expandable batch size in producer

2021-10-20 Thread Luke Chen
Hi Ismael and all devs,
Is there any comments/suggestions to this KIP?
If no, I'm going to update the KIP based on my previous mail, and start a
vote tomorrow or next week.

Thank you.
Luke

On Mon, Oct 18, 2021 at 2:40 PM Luke Chen  wrote:

> Hi Ismael,
> Thanks for your comments.
>
> 1. Why do we have to reallocate the buffer? We can keep a list of buffers
> instead and avoid reallocation.
> -> Do you mean we allocate multiple buffers with "buffer.initial.size",
> and link them together (with linked list)?
> ex:
> a. We allocate 4KB initial buffer
> | 4KB |
>
> b. when new records reached and the remaining buffer is not enough for the
> records, we create another batch with "batch.initial.size" buffer
> ex: we already have 3KB of data in the 1st buffer, and here comes the 2KB
> record
>
> | 4KB (1KB remaining) |
> now, record: 2KB coming
> We fill the 1st 1KB into 1st buffer, and create new buffer, and linked
> together, and fill the rest of data into it
> | 4KB (full) | ---> | 4KB (3KB remaining) |
>
> Is that what you mean?
> If so, I think I like this idea!
> If not, please explain more detail about it.
> Thank you.
>
> 2. I think we should also consider tweaking the semantics of batch.size so
> that the sent batches can be larger if the batch is not ready to be sent
> (while still respecting max.request.size and perhaps a new max.batch.size).
>
> --> In the KIP, I was trying to make the "batch.size" as the upper bound
> of the batch size, and introduce a "batch.initial.size" as initial batch
> size.
> So are you saying that we can let "batch.size" as initial batch size and
> introduce a "max.batch.size" as upper bound value?
> That's a good suggestion, but that would change the semantics of
> "batch.size", which might surprise some users. I think my original proposal
> ("batch.initial.size") is safer for users. What do you think?
>
> Thank you.
> Luke
>
>
> On Mon, Oct 18, 2021 at 3:12 AM Ismael Juma  wrote:
>
>> I think we should also consider tweaking the semantics of batch.size so
>> that the sent batches can be larger if the batch is not ready to be sent
>> (while still respecting max.request.size and perhaps a new
>> max.batch.size).
>>
>> Ismael
>>
>> On Sun, Oct 17, 2021, 12:08 PM Ismael Juma  wrote:
>>
>> > Hi Luke,
>> >
>> > Thanks for the KIP. Why do we have to reallocate the buffer? We can
>> keep a
>> > list of buffers instead and avoid reallocation.
>> >
>> > Ismael
>> >
>> > On Sun, Oct 17, 2021, 2:02 AM Luke Chen  wrote:
>> >
>> >> Hi Kafka dev,
>> >> I'd like to start the discussion for the proposal: KIP-782: Expandable
>> >> batch size in producer.
>> >>
>> >> The main purpose for this KIP is to have better memory usage in
>> producer,
>> >> and also save users from the dilemma while setting the batch size
>> >> configuration. After this KIP, users can set a higher batch.size
>> without
>> >> worries, and of course, with an appropriate "batch.initial.size" and
>> >> "batch.reallocation.factor".
>> >>
>> >> Derailed description can be found here:
>> >>
>> >>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
>> >>
>> >> Any comments and feedback are welcome.
>> >>
>> >> Thank you.
>> >> Luke
>> >>
>> >
>>
>


[jira] [Resolved] (KAFKA-13372) failed authentication due to: SSL handshake failed

2021-10-18 Thread Luke Chen (Jira)


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

Luke Chen resolved KAFKA-13372.
---
Resolution: Resolved

> failed authentication due to: SSL handshake failed
> --
>
> Key: KAFKA-13372
> URL: https://issues.apache.org/jira/browse/KAFKA-13372
> Project: Kafka
>  Issue Type: Bug
>  Components: clients
>Affects Versions: 2.2.2
>Reporter: Maria Isabel Florez Rodriguez
>Priority: Major
>
> Hi everyone,
>  
> I have the next issue about authentication SCRAM + SSL. I’m using the CLI and 
> this is the version of my client (./kafka_2.13-2.8.1/bin/kafka-topics.sh). In 
> this example I will talk about list topics, but another operations (consumer, 
> producer) failed too.
>  
>  
> First, let me describe the current scenario:
>  
>  * I have 5 Kafka servers with 
>  * kafka-broker-0.mydomain.com
>  * kafka-broker-1.mydomain.com
>  * kafka-broker-2.mydomain.com
>  * kafka-broker-3.mydomain.com
>  * kafka-broker-4.mydomain.com
>  
>  * I have a DNS principal configured with Round Robin to IPs broker:
>  * kafka-broker-princial.mydomain.com (Round Robin)
>  
>  I have configured for each broker the next listeners (I'm using 3 ports):
> {quote}advertised.listeners=SASL_SSL://kafka-broker-0.mydomain.com:9094,SASL_PLAINTEXT://kafka-broker-0.mydomain.com:9093,PLAINTEXT://kafka-broker-0.mydomain.com:9092{quote}
>  * 9092 for PLAINTEXT
>  * 9093 for SASL_PLAINTEXT
>  * 9094 for SASL_SSL
>  
> My Kafka broker servers have the next config server.properties:
> {quote}advertised.listeners=SASL_SSL://kafka-broker-X.mydomain.com:9094,SASL_PLAINTEXT://kafka-broker-X.mydomain.com:9093,PLAINTEXT://kafka-broker-X.mydomain.com:9092
> authorizer.class.name=kafka.security.auth.SimpleAclAuthorizer
> auto.create.topics.enable=false
> auto.leader.rebalance.enable=true
> background.threads=10
> broker.id=X
> broker.rack=us-east-1c
> compression.type=producer
> connections.max.idle.ms=270
> controlled.shutdown.enable=true
> delete.topic.enable=true
> host.name=localhost
> leader.imbalance.check.interval.seconds=300
> leader.imbalance.per.broker.percentage=10
> listeners=SASL_SSL://0.0.0.0:9094,SASL_PLAINTEXT://0.0.0.0:9093,PLAINTEXT://0.0.0.0:9092
> log.cleaner.enable=true
> log.dirs=/var/lib/kafka/log/data1,/var/lib/kafka/log/data2,/var/lib/kafka/log/data3
> log.retention.check.interval.ms=30
> log.retention.hours=336
> log.segment.bytes=1073741824
> message.max.bytes=112
> min.insync.replicas=2
> num.io.threads=8
> num.network.threads=3
> num.partitions=3
> num.recovery.threads.per.data.dir=1
> num.replica.fetchers=1
> offset.metadata.max.bytes=4096
> offsets.commit.timeout.ms=5000
> offsets.retention.minutes=129600
> offsets.topic.num.partitions=50
> offsets.topic.replication.factor=3
> port=9092
> queued.max.requests=500
> replica.fetch.min.bytes=1
> replica.fetch.wait.max.ms=500
> sasl.enabled.mechanisms=SCRAM-SHA-256,GSSAPI
> sasl.kerberos.service.name=x
> sasl.mechanism.inter.broker.protocol=SCRAM-SHA-256
> security.inter.broker.protocol=SASL_SSL
> socket.receive.buffer.bytes=102400
> socket.request.max.bytes=104857600
> socket.send.buffer.bytes=102400
> ssl.client.auth=required
> {{ssl.endpoint.identification.algorithm=""}}
> ssl.enabled.protocols=TLSv1.2
> ssl.key.password=
> ssl.keystore.location=/etc/ssl/default_keystore.jks
> ssl.keystore.password=
> ssl.truststore.location=/usr/lib/jvm/java-11-adoptopenjdk-hotspot/lib/security/cacerts
> ssl.truststore.password= 
> ssl.truststore.type=JKS
> super.users=User:x
> zookeeper.connect=kafka-zk-X.mydomain.com:2181,kafka-zk-X.mydomain.com:2181,kafka-zk-X.mydomain.com:2181,kafka-zk-X.mydomain.com
>  :2181,kafka-zk-X.mydomain.com:218/my-environment
> zookeeper.connection.timeout.ms=6000
> zookeeper.sasl.client=false{quote}
>  
>  
> I was trying the next things:
>  
>  * (/)*PLAINTEXT:* I can consume directly to broker to broker with port 
> *9092* (Using IP or dns broker) 
>  * (/)*PLAINTEXT:* I also can consume directly to DNS principal configured 
> with Round Robin  with port *9092* (Using DNS principal)
>  * (/)*SASL_SSL:* I can consume directly to broker to broker with port *9094* 
> (Using only dns broker due it needs to validate the certificate)
>  * (x)*SASL_SSL:* I cannot consume directly to DNS principal configured with 
> Round Robin with port *9094*
> The issue is: * *(x)SASL_SSL(x):* I cannot consume directly to DNS principal 
> configured with R

Re: [VOTE] KIP-764 Configurable backlog size for creating Acceptor

2021-10-18 Thread Luke Chen
Hi Okada,
Thanks for the KIP.
+1 (non-binding)

One thing to add is that you should add ServerSocket#bind java doc link
into the KIP.
I don't think everyone is familiar with the definition of the method
parameters.

Thank you.
Luke

On Mon, Oct 18, 2021 at 3:43 PM Haruki Okada  wrote:

> Hi Kafka.
>
> Let me bump this VOTE thread for the KIP.
> We applied proposed changes in the KIP to our large Kafka cluster by
> building patched Kafka internally and confirmed it's working well.
>
> Please feel free to give your feedback if there's any points to be
> clarified in the KIP.
>
> Thanks,
>
> 2021年8月9日(月) 11:25 Haruki Okada :
>
> > Thanks for your comment LI-san.
> >
> > Could anyone else review and vote for the KIP?
> >
> > I think the situation described in the KIP's motivation can happen in any
> > large-scale Kafka deployment, so may be helpful for many users while the
> > proposed changes are small enough.
> >
> >
> > Thanks,
> >
> > 2021年8月3日(火) 15:49 Xiangyuan LI :
> >
> >> Hi Haruki Okada:
> >>   i read your comment, thx for your detail explain!
> >>   add backlog parameter is a useful suggestion, hope it could added to
> >> kafka.
> >>
> >> Haruki Okada  于2021年8月2日周一 上午7:43写道:
> >>
> >> > Hi, Kafka.
> >> >
> >> > I would like to start a vote on KIP that makes SocketServer acceptor's
> >> > backlog size configurable.
> >> >
> >> > KIP:
> >> >
> >> >
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-764%3A+Configurable+backlog+size+for+creating+Acceptor
> >> >
> >> > Discussion thread:
> >> >
> >> >
> >>
> https://lists.apache.org/thread.html/rd77469b7de0190d601dd37bd6894e1352a674d08038bcfe7ff68a1e0%40%3Cdev.kafka.apache.org%3E
> >> >
> >> > Thanks,
> >> >
> >> > --
> >> > 
> >> > Okada Haruki
> >> > ocadar...@gmail.com
> >> > 
> >> >
> >>
> >
> >
> > --
> > 
> > Okada Haruki
> > ocadar...@gmail.com
> > 
> >
>
>
> --
> 
> Okada Haruki
> ocadar...@gmail.com
> 
>


Re: [VOTE] Add TaskId field to StreamsException

2021-10-18 Thread Luke Chen
Hi Sophie,
Add taskId to make the exception much clear is a good improvement.
+ 1 (non-binding)

Thank you.
Luke

On Mon, Oct 18, 2021 at 12:10 PM Sophie Blee-Goldman
 wrote:

> Hey all,
>
> I'd like to kick off the vote on this small KIP which adds a TaskId field
> to the StreamsException class. Please take a look and cast your vote when
> you have a chance.
>
> Links:
>
>- KIP-783: Add TaskId field to StreamsException
>
>- PR #11405 
>
>
> Thanks!
> Sophie
>


Re: [DISCUSS] KIP-782: Expandable batch size in producer

2021-10-18 Thread Luke Chen
Hi Ismael,
Thanks for your comments.

1. Why do we have to reallocate the buffer? We can keep a list of buffers
instead and avoid reallocation.
-> Do you mean we allocate multiple buffers with "buffer.initial.size", and
link them together (with linked list)?
ex:
a. We allocate 4KB initial buffer
| 4KB |

b. when new records reached and the remaining buffer is not enough for the
records, we create another batch with "batch.initial.size" buffer
ex: we already have 3KB of data in the 1st buffer, and here comes the 2KB
record

| 4KB (1KB remaining) |
now, record: 2KB coming
We fill the 1st 1KB into 1st buffer, and create new buffer, and linked
together, and fill the rest of data into it
| 4KB (full) | ---> | 4KB (3KB remaining) |

Is that what you mean?
If so, I think I like this idea!
If not, please explain more detail about it.
Thank you.

2. I think we should also consider tweaking the semantics of batch.size so
that the sent batches can be larger if the batch is not ready to be sent
(while still respecting max.request.size and perhaps a new max.batch.size).

--> In the KIP, I was trying to make the "batch.size" as the upper bound of
the batch size, and introduce a "batch.initial.size" as initial batch size.
So are you saying that we can let "batch.size" as initial batch size and
introduce a "max.batch.size" as upper bound value?
That's a good suggestion, but that would change the semantics of
"batch.size", which might surprise some users. I think my original proposal
("batch.initial.size") is safer for users. What do you think?

Thank you.
Luke


On Mon, Oct 18, 2021 at 3:12 AM Ismael Juma  wrote:

> I think we should also consider tweaking the semantics of batch.size so
> that the sent batches can be larger if the batch is not ready to be sent
> (while still respecting max.request.size and perhaps a new max.batch.size).
>
> Ismael
>
> On Sun, Oct 17, 2021, 12:08 PM Ismael Juma  wrote:
>
> > Hi Luke,
> >
> > Thanks for the KIP. Why do we have to reallocate the buffer? We can keep
> a
> > list of buffers instead and avoid reallocation.
> >
> > Ismael
> >
> > On Sun, Oct 17, 2021, 2:02 AM Luke Chen  wrote:
> >
> >> Hi Kafka dev,
> >> I'd like to start the discussion for the proposal: KIP-782: Expandable
> >> batch size in producer.
> >>
> >> The main purpose for this KIP is to have better memory usage in
> producer,
> >> and also save users from the dilemma while setting the batch size
> >> configuration. After this KIP, users can set a higher batch.size without
> >> worries, and of course, with an appropriate "batch.initial.size" and
> >> "batch.reallocation.factor".
> >>
> >> Derailed description can be found here:
> >>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer
> >>
> >> Any comments and feedback are welcome.
> >>
> >> Thank you.
> >> Luke
> >>
> >
>


[DISCUSS] KIP-782: Expandable batch size in producer

2021-10-17 Thread Luke Chen
Hi Kafka dev,
I'd like to start the discussion for the proposal: KIP-782: Expandable
batch size in producer.

The main purpose for this KIP is to have better memory usage in producer,
and also save users from the dilemma while setting the batch size
configuration. After this KIP, users can set a higher batch.size without
worries, and of course, with an appropriate "batch.initial.size" and
"batch.reallocation.factor".

Derailed description can be found here:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-782%3A+Expandable+batch+size+in+producer

Any comments and feedback are welcome.

Thank you.
Luke


Re: [SPAM] Re: Why does Kafka have a higher throughput than Redis?

2021-10-14 Thread Luke Chen
Hi Vitor,
I'm not the expert, either, but I think Andrew's answer is pretty much the
reasons why Kafka is doing good.
And I'm not too familiar with Redis, either. But I'd say, there are many
configurations in each product to increase the throughput, and the use
cases are different, the comparison might not be fair.

For your question:
3.  Didn't know about this zero-copy technique, I'll read more about that
but feels like the result would be a response similar to as if kafka had
the info stored in-memory (as redis do) but that would still make me
question how is that Kafka can handle a higher throughput if the "design"
is so similar.
--> Again, I'm not familiar with Redis, but even you store data in memory,
if there's no OS's help, you still need to copy data to kernel space to
send to the receiver, compared with the zero-copy technique, all data flow
are within kernel space.

But again, the use cases are different, the comparison might not be fair.
We can only analyze and learn why and how they have good throughput.
That's my two cents.

Thank you.
Luke

On Fri, Oct 15, 2021 at 3:47 AM Vitor Augusto de Medeiros <
v.medei...@aluno.ufabc.edu.br> wrote:

> Thanks for the response, Andrew, i appreciate the help!
>
> Just i few thoughts that came up while reading your points:
>
>
>   1.  In theory, Redis is also handling/storing data in memory which makes
> me wonder why is that Kafka does it better? Perhaps it has to do with the
> API contract, where, as you said, there's no complex transactional software
> that might hurt performance.
>   2.  Didn't know there was such a big difference from linear to random
> writes, pretty awesome! But I still don't understand how disk usage, even
> If doing linear writes, is still allowing a throughput rate of 2 to 3x the
> amount of Redis, which doesn't use disk write/read at all and keep messages
> stored in memory.
>   3.  Didn't know about this zero-copy technique, I'll read more about
> that but feels like the result would be a response similar to as if kafka
> had the info stored in-memory (as redis do) but that would still make me
> question how is that Kafka can handle a higher throughput if the "design"
> is so similar.
>
>
> 
> De: Andrew Grant 
> Enviado: quinta-feira, 14 de outubro de 2021 15:55
> Para: dev@kafka.apache.org 
> Assunto: [SPAM] Re: Why does Kafka have a higher throughput than Redis?
>
> Hi Vitor,
>
> I'm not an expert and probably some more knowledgeable folks can also chime
> in (and correct me) but a few things came to mind:
>
> 1) On the write side (i.e. when using the publisher), Kafka does not flush
> data to disk by default. It writes to the page cache so all writes are sort
> of in-memory in a way. They're staged in the page cache and the kernel
> flushes the data asynchronously. Also the API contract for Kafka is quite
> "simple" in that it mostly reads and writes arbitrary sequences of bytes -
> there isn't as much complex transactional software in front of the
> writing/reading that might hurt performance compared to some other data
> stores. Note, Kafka does provide things like idempotence and transactions
> so it's not like there is never any overhead to consider.
>
> 2) Kafka reads and writes are conducive to being linear which helps a lot
> with performance. Random writes are a lot slower than linear ones.
>
> 3) For reading (i.e. when using the consumer) data Kafka uses a zero-copy
> technique in which data is directly sent from the page cache to the network
> buffer without going through user space which helps a lot.
>
> 4) Kafka batches aggressively.
>
> Here are two resources which might provide more information
> https://docs.confluent.io/platform/current/kafka/design.html,
>
> https://engineering.linkedin.com/kafka/benchmarking-apache-kafka-2-million-writes-second-three-cheap-machines
> .
>
> Hope this helps a bit.
>
> Andrew
>
> On Thu, Oct 14, 2021 at 1:11 PM Vitor Augusto de Medeiros <
> v.medei...@aluno.ufabc.edu.br> wrote:
>
> > Hi everyone,
> >
> >  i'm doing a benchmark comparison between Kafka and Redis for my final
> > bachelor paper and would like to understand more about why Kafka have
> > higher throughput if compared to Redis.
> >
> >  I noticed Redis has lower overall latency (and makes sense since it's
> > stored in memory) but cant figure out the difference in throughput.
> >
> > I found a study (not sure if i can post links here but it's named A
> > COMPARISON OF DATA INGESTION PLATFORMS IN REAL-TIME STREAM PROCESSING
> > PIPELINES by Sebastian Tallberg)
> > showing Kafka's throughput hitting 3x the amount of msg/s if compared to
> > Redis for a 1kB payload. I would like to understand what is in Kafka's
> > architecture that allows it to be a lot faster than other message
> > brokers/Redis in particular
> >
> > Thanks!
> >
>
>
> --
> Andrew Grant
> 8054482621
>


Re: Kafka client 2.7.1 missing JaasUtils.isZkSecurityEnabled() method

2021-10-14 Thread Luke Chen
Hi Alexandre
Yes, you're right. We renamed the `isZkSecurityEnabled` method name into
`isZkSaslEnabled`, because it checked sasl config only. You can check here

.

If you want to check TLS configuration, you can check here

and here
,
basically it just check the tls is enabled and client socket/keystore is
set.

Thank you.
Luke


On Thu, Oct 14, 2021 at 11:04 PM Alexandre Vermeerbergen <
avermeerber...@gmail.com> wrote:

> Hello,
>
> When upgrading from Kafka client 2.4.0 our dependencies to Kafka
> client 2.7.1, we noticed that JaasUtils.isZkSecurityEnabled() method
> no longer exists.
>
> Is there an equivalent method in Kafka client 2.7.1 which could use
> instead ?
>
> Kind regards,
> Alexandre
>


Re: [DISCUSS] Apache Kafka 3.1.0 release

2021-10-14 Thread Luke Chen
Hi David,
KIP-766 is merged into trunk. Please help add it into the release plan.

Thank you.
Luke

On Mon, Oct 11, 2021 at 10:50 PM David Jacot 
wrote:

> Hi Michael,
>
> Sure. I have updated the release plan to include it. Thanks for the
> heads up.
>
> Best,
> David
>
> On Mon, Oct 11, 2021 at 4:39 PM Mickael Maison 
> wrote:
>
> > Hi David,
> >
> > You can add KIP-690 to the release plan. The vote passed months ago
> > and I merged the PR today.
> >
> > Thanks
> >
> > On Fri, Oct 8, 2021 at 8:32 AM David Jacot 
> > wrote:
> > >
> > > Hi folks,
> > >
> > > Just a quick reminder that KIP Freeze is next Friday, October 15th.
> > >
> > > Cheers,
> > > David
> > >
> > > On Wed, Sep 29, 2021 at 3:52 PM Chris Egerton
> > 
> > > wrote:
> > >
> > > > Thanks David!
> > > >
> > > > On Wed, Sep 29, 2021 at 2:56 AM David Jacot
> > 
> > > > wrote:
> > > >
> > > > > Hi Chris,
> > > > >
> > > > > Sure thing. I have added KIP-618 to the release plan. Thanks for
> the
> > > > heads
> > > > > up.
> > > > >
> > > > > Best,
> > > > > David
> > > > >
> > > > > On Wed, Sep 29, 2021 at 8:53 AM David Jacot 
> > wrote:
> > > > >
> > > > > > Hi Kirk,
> > > > > >
> > > > > > Yes, it is definitely possible if you can get the KIP voted
> before
> > the
> > > > > KIP
> > > > > > freeze
> > > > > > and the code committed before the feature freeze. Please, let me
> > know
> > > > > when
> > > > > > the
> > > > > > KIP is voted and I will add it to the release plan.
> > > > > >
> > > > > > Thanks,
> > > > > > David
> > > > > >
> > > > > > On Tue, Sep 28, 2021 at 7:05 PM Chris Egerton
> > > > > 
> > > > > > wrote:
> > > > > >
> > > > > >> Hi David,
> > > > > >>
> > > > > >> Wondering if we can get KIP-618 included? The vote passed months
> > ago
> > > > > and a
> > > > > >> PR has been available since mid-June.
> > > > > >>
> > > > > >> Cheers,
> > > > > >>
> > > > > >> Chris
> > > > > >>
> > > > > >> On Tue, Sep 28, 2021 at 12:53 PM Kirk True <
> k...@mustardgrain.com
> > >
> > > > > wrote:
> > > > > >>
> > > > > >> > Hi David,
> > > > > >> >
> > > > > >> > Is it possible to try to get KIP-768 in 3.1? I have put it up
> > for a
> > > > > vote
> > > > > >> > and have much of it implemented already.
> > > > > >> >
> > > > > >> > Thanks,
> > > > > >> > Kirk
> > > > > >> >
> > > > > >> > On Tue, Sep 28, 2021, at 3:11 AM, Israel Ekpo wrote:
> > > > > >> > > Ok. Sounds good, David.
> > > > > >> > >
> > > > > >> > > Let’s forge ahead. The plan looks good.
> > > > > >> > >
> > > > > >> > > On Tue, Sep 28, 2021 at 4:02 AM David Jacot
> > > > > >>  > > > > >> > >
> > > > > >> > > wrote:
> > > > > >> > >
> > > > > >> > > > Hi Israel,
> > > > > >> > > >
> > > > > >> > > > Yeah, 3.0 took quite a long time to be released. However,
> I
> > > > think
> > > > > >> > > > that we should stick to our time based release.
> > > > > >> > > >
> > > > > >> > > > Best,
> > > > > >> > > > David
> > > > > >> > > >
> > > > > >> > > >
> > > > > >> > > > On Tue, Sep 28, 2021 at 9:59 AM David Jacot <
> > > > dja...@confluent.io>
> > > > > >> > wrote:
> > > > > >> > > >
> > > > > >> > > > > Hi Bruno,
> > > > > >> > > > >
> > > > > >> > > > > Thanks for the heads up. I have removed it from the
> plan.
> > > > > >> > > > >
> > > > > >> > > > > Best,
> > > > > >> > > > > David
> > > > > >> > > > >
> > > > > >> > > > > On Mon, Sep 27, 2021 at 11:04 AM Bruno Cadonna <
> > > > > >> cado...@apache.org>
> > > > > >> > > > wrote:
> > > > > >> > > > >
> > > > > >> > > > >> Hi David,
> > > > > >> > > > >>
> > > > > >> > > > >> Thank you for the plan!
> > > > > >> > > > >>
> > > > > >> > > > >> KIP-698 will not make it for 3.1.0. Could you please
> > remove
> > > > it
> > > > > >> from
> > > > > >> > the
> > > > > >> > > > >> plan?
> > > > > >> > > > >>
> > > > > >> > > > >> Best,
> > > > > >> > > > >> Bruno
> > > > > >> > > > >>
> > > > > >> > > > >> On 24.09.21 16:22, David Jacot wrote:
> > > > > >> > > > >> > Hi all,
> > > > > >> > > > >> >
> > > > > >> > > > >> > I just published a release plan here:
> > > > > >> > > > >> >
> > > > > >> >
> > > > https://cwiki.apache.org/confluence/display/KAFKA/Release+Plan+3.1.0
> > > > > >> > > > >> >
> > > > > >> > > > >> > The plan suggests the following dates:
> > > > > >> > > > >> >
> > > > > >> > > > >> > KIP Freeze: 15 October 2021
> > > > > >> > > > >> > Feature Freeze: 29 October 2021
> > > > > >> > > > >> > Code Freeze: 12 November 2021
> > > > > >> > > > >> >
> > > > > >> > > > >> > At least two weeks of stabilization will follow Code
> > > > Freeze.
> > > > > >> > > > >> >
> > > > > >> > > > >> > I have included all the currently approved KIPs
> > targeting
> > > > > >> 3.1.0.
> > > > > >> > > > Please
> > > > > >> > > > >> > let me know if I should add/remove any to/from the
> > plan.
> > > > > >> > > > >> >
> > > > > >> > > > >> > Please let me know if you have any objections.
> > > > > >> > > > >> >
> > > > > >> > > > >> > Regards,
> > > > > >> > > > >> > David
> > > > > >> > > > >> >
> > > > > >> > > > >> > On Mon, Sep 20, 

Re: [DISCUSS] KIP-780: Support fine-grained compression options

2021-10-13 Thread Luke Chen
Hi Dongjin,
Thanks for the KIP, and the benchmark results. It makes sense to me.

Just one question:
> compression.zstd.window: enables long mode; the log of the window size
that zstd uses to memorize the compressing data. (available: [10, 22],
default: 0 (disables long mode.))

It said, available value is [10, 22], but default is a value out of that
range, which should be wrong.

Thank you.
Luke

On Sun, Oct 10, 2021 at 9:50 PM Dongjin Lee  wrote:

> Hi Kafka dev,
>
> I would like to start the discussion of KIP-780: Support fine-grained
> compression options.
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-780%3A+Support+fine-grained+compression+options
>
> Here is some context or history on this feature; initially, this feature
> was intended to be a part of KIP-390: Support Compression Level, but when I
> was working on it, I could not find the evidence that these options can
> improve the performance, so it was excluded from the final proposal. Since
> this (tentative) conclusion was somewhat strange, KIP-390 was passed under
> the condition that a following work should be done for the
> buffer/block/window-related configuration options.
>
> And after some repetitive prototypes and benchmarks, it seems like I
> finally found the evidence. It is why I am submitting it as a separate
> proposal now. The document also includes what I found during the tests in
> the Benchmark section.
>
> All kinds of feedbacks are greatly appreciated!
>
> Best,
> Dongjin
>
> --
> *Dongjin Lee*
>
> *A hitchhiker in the mathematical world.*
>
>
>
> *github:  github.com/dongjinleekr
> keybase: https://keybase.io/dongjinleekr
> linkedin: kr.linkedin.com/in/dongjinleekr
> speakerdeck:
> speakerdeck.com/dongjin
> *
>


Re: Why need handle delete topic in topic change event

2021-10-11 Thread Luke Chen
Hi 晓兵,
I know what you mean now.
Yes, that makes sense to me, as long as you confirmed that each topic
deletion, we'll put a znode under "delete_topics".
Please open a jira ticket and welcome to submit PR.

Thank you.
Luke

On Tue, Oct 12, 2021 at 10:32 AM 方晓兵 <94fxiaob...@gmail.com> wrote:

> Hi Luke,
> What I mean is whether there is no need to listen to the topic znode
> deletion event, because `controllerContext#removeTopic` has been actively
> called after deleting znode in
> `TopicDeletionManager.completeDeleteTopic()`. Is it better for
> TopicChangeHandler to only handle child add events?
>
> > 2021年10月12日 上午10:23,Luke Chen  写道:
> >
> > Hi 晓兵,
> > Are you saying we should not call `removeTopic` because the topic znode
> is
> > deleted?
> > Have a quick look at the `controllerContext#removeTopic` implementation,
> it
> > looks like we only clean up the metrics, and local maps. Why is it a
> > problem we called it here? Does it caused any error?
> > Could you elaborate it more?
> >
> > Thank you.
> > Luke
> >
> > On Mon, Oct 11, 2021 at 6:10 PM 方晓兵 <94fxiaob...@gmail.com> wrote:
> >
> >> Hi Team,
> >>
> >> I have a problem when I study kafka code in version 2.8.0.
> >>
> >> I see `controllerContext.removeTopic(topic)` have been called in
> >> `TopicDeletionManager.completeDeleteTopic()`. TopicChangeHandler not
> only
> >> listen to child delete but also listen to child add. So every time a
> topic
> >> is deleted, an invalid event will be triggered because
> >> `controllerContext.removeTopic(topic)` have been called in
> >> `TopicDeletionManager.completeDeleteTopic()` after delete this topic
> >> zookeeper path. Is it code that needs to be optimized?
> >>
> >> Grateful and look forward to answers
>
>


Re: Why need handle delete topic in topic change event

2021-10-11 Thread Luke Chen
Hi 晓兵,
Are you saying we should not call `removeTopic` because the topic znode is
deleted?
Have a quick look at the `controllerContext#removeTopic` implementation, it
looks like we only clean up the metrics, and local maps. Why is it a
problem we called it here? Does it caused any error?
Could you elaborate it more?

Thank you.
Luke

On Mon, Oct 11, 2021 at 6:10 PM 方晓兵 <94fxiaob...@gmail.com> wrote:

> Hi Team,
>
> I have a problem when I study kafka code in version 2.8.0.
>
> I see `controllerContext.removeTopic(topic)` have been called in
> `TopicDeletionManager.completeDeleteTopic()`. TopicChangeHandler not only
> listen to child delete but also listen to child add. So every time a topic
> is deleted, an invalid event will be triggered because
> `controllerContext.removeTopic(topic)` have been called in
> `TopicDeletionManager.completeDeleteTopic()` after delete this topic
> zookeeper path. Is it code that needs to be optimized?
>
> Grateful and look forward to answers


Re: [DISCUSS] KIP-776: Add Consumer#peek for debugging/tuning

2021-10-05 Thread Luke Chen
Hi Mickael,
Thanks for your comments.
I've added a use case into the KIP, and added Boyang's comment into
rejected alternatives section.

Hope that makes sense and strengthens the motivation.
If you have other suggestions, please let me know.

Thank you.
Luke

On Mon, Oct 4, 2021 at 10:23 PM Mickael Maison 
wrote:

> Hi Luke,
>
> Thanks for the KIP.
>
> Can you clarify the use cases you have in mind exactly? This would
> strengthen the motivation which I find a bit weak at the moment.
>
> As mentioned by Boyang, it's possible to achieve something similar
> with the existing APIs, for example with poll/seek or with
> listOffsets. Can you list them in the rejected alternatives section
> and explain why they were rejected.
>
> Thanks
>
>
> On Tue, Sep 21, 2021 at 9:47 AM Luke Chen  wrote:
> >
> > Thanks for your feedback, Sagar, Boyang.
> >
> > I've added an additional API to take the Set as the
> > partitions to fetch from. Good suggestion!
> > I also updated the java doc in the KIP.
> >
> > And for the question that the behavior can also be achieved by using
> manual
> > offset commit + offset position rewind. That's true.
> > But I have the same thoughts as Sagar, which is that, it's for advanced
> > users.
> > Another reason is for simplicity. If you've ever used the peek API from
> > java collection (ex: Queue#peek), you should know what I'm talking about.
> > When you have data in a queue, if you want to know what the first data is
> > in the queue, you'd use peek(). You can also achieve it by remove() the
> 1st
> > element from queue, and then added it back to the right position, but I
> > believe that's not what you'd do.
> >
> > Thank you.
> > Luke
> >
> >
> > On Tue, Sep 21, 2021 at 1:02 AM Sagar  wrote:
> >
> > > Thanks Luke for the KIP. I think it makes sense.
> > >
> > > @Boyang,
> > >
> > > While it is possible to get the functionality using manual offset
> commit +
> > > offset position rewind as you stated, IMHO it could still be a very
> handy
> > > addition to the APIs.  The way I see it, manual offset commit + offset
> > > position rewind is for slightly more advanced users and the addition of
> > > peek() API would make it trivial to get the mentioned functionality.
> > >
> > > I agree to the point of adding a mechanism to fetch a more fine
> grained set
> > > of records. Maybe, add another API which takes a Set?
> In
> > > this case, we would probably need to add a behaviour to throw some
> > > exception when a user tries to peek from a TopicPartition that he/she
> isn't
> > > subscribed to.
> > >
> > > nit: In the javadoc, this line =>
> > >
> > > This method returns immediately if there are records available or
> > > exception thrown.
> > >
> > >
> > > should probably be =>
> > >
> > >
> > > This method returns immediately if there are no records available or
> > > exception thrown.
> > >
> > >
> > > Thanks!
> > >
> > > Sagar.
> > >
> > >
> > >
> > >
> > > On Mon, Sep 20, 2021 at 4:22 AM Boyang Chen <
> reluctanthero...@gmail.com>
> > > wrote:
> > >
> > > > Thanks Luke for the KIP.
> > > >
> > > > I think I understand the motivation is to avoid affecting offset
> > > positions
> > > > of the records, but the feature could be easily realized on the user
> side
> > > > by using manual offset commit + offset position rewind. So the new
> peek()
> > > > function doesn't provide any new functionality IMHO, weakening the
> > > > motivation a bit.
> > > >
> > > > Additionally, for the peek() case, I believe that users may want to
> have
> > > > more fine-grained exposure of records, such as from specific
> partitions
> > > > instead of getting random records. It's probably useful to define an
> > > option
> > > > handle class in the parameters to help clarify what specific records
> to
> > > be
> > > > returned.
> > > >
> > > > Boyang
> > > >
> > > > On Sun, Sep 19, 2021 at 1:51 AM Luke Chen  wrote:
> > > >
> > > > > Hi everyone,
> > > > >
> > > > > I'd like to discuss the following proposal to add Consumer#peek for
> > > > > debugging/tuning.
> > > > >
> > > > > The main purpose for Consumer#peek is to allow users:
> > > > >
> > > > >1. peek what records existed at broker side and not increasing
> the
> > > > >position offsets.
> > > > >2. throw exceptions when there is connection error existed
> between
> > > > >consumer and broker (or other exceptions will be thrown by
> "poll")
> > > > >
> > > > >
> > > > > detailed description can be found her:
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=188746244
> > > > >
> > > > >
> > > > > Any comments and feedback are welcomed.
> > > > >
> > > > > Thank you.
> > > > > Luke
> > > > >
> > > >
> > >
>


[VOTE] KIP-776: Add Consumer#peek for debugging/tuning

2021-10-04 Thread Luke Chen
Hi everyone,

I'd like to start a vote for the following proposal to add Consumer#peek
for debugging/tuning.

The main purpose for Consumer#peek is to allow users:

   1. peek what records existed at broker side and not increasing the
   position offsets.
   2. throw exceptions when there is connection error existed between
   consumer and broker (or other exceptions will be thrown by "poll")


Detailed description can be found here:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=188746244

Thank you
Luke


<    3   4   5   6   7   8   9   10   >