Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-15 Thread Guozhang Wang
Yes, I will update the KIP accordingly. Thanks. On Tue, Nov 14, 2017 at 2:56 PM, Matthias J. Sax wrote: > One more thing. Can you update the KIP accordingly. It still says: > > - Compatibility check: we will use a network client for this purpose, > as it is a one-time

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-14 Thread Matthias J. Sax
One more thing. Can you update the KIP accordingly. It still says: - Compatibility check: we will use a network client for this purpose, as it is a one-time thing. Additionally, I think we should add a "admin." prefix that allows to set certain config parameters for the admin client only.

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-14 Thread Matthias J. Sax
Thanks for looking into this into details! As mentioned, I would like to keep the check, but if it's too much overhead, I agree that it's not worth it. Thanks. -Matthias On 11/14/17 10:00 AM, Guozhang Wang wrote: > I looked into how to use a NetworkClient to replace StreamsKafkaClient to > do

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-14 Thread Guozhang Wang
I looked into how to use a NetworkClient to replace StreamsKafkaClient to do this one-time checking, and the complexity is actually pretty high: since it is a barebone NetworkClient, we have to handle the connection / readiness / find a broker to send to / etc logic plus introducing all these

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-10 Thread Bill Bejeck
I'm leaning towards option 3, although option 2 is a reasonable tradeoff between the two. Overall I'm leaning towards option 3 because: 1. As Guozhang has said we are failing "fast enough" with an Exception from the first rebalance. 2. Less complexity/maintenance cost by not having a

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-07 Thread Guozhang Wang
Here is what I'm thinking about this trade-off: we want to fail fast when brokers do not yet support the requested API version, with the cost that we need to do this one-time thing with an expensed NetworkClient plus a bit longer starting up latency. Here are a few different options: 1) we ask

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-07 Thread Matthias J. Sax
I would prefer to keep the current check. We could even improve it, and do the check to more than one brokers (even all provided bootstrap.servers) or some random servers after we got all meta data about the cluster. -Matthias On 11/7/17 1:01 AM, Guozhang Wang wrote: > Hello folks, > > One

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-06 Thread Guozhang Wang
Hello folks, One update I'd like to propose regarding "compatibility checking": currently we create a single StreamsKafkaClient at the beginning to issue an ApiVersionsRequest to a random broker and then check on its versions, and fail if it does not satisfy the version (0.10.1+ without EOS,

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-06 Thread Matthias J. Sax
Thanks for the update and clarification. Sounds good to me :) -Matthias On 11/6/17 12:16 AM, Guozhang Wang wrote: > Thanks Matthias, > > 1) Updated the KIP page to include KAFKA-6126. > 2) For passing configs, I agree, will make a pass over the existing configs > passed to

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-05 Thread Guozhang Wang
Thanks Matthias, 1) Updated the KIP page to include KAFKA-6126. 2) For passing configs, I agree, will make a pass over the existing configs passed to StreamsKafkaClient and update the wiki page accordingly, to capture all changes that would happen for the replacement in this single KIP. 3) For

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-05 Thread Guozhang Wang
Thanks Ted, has updated the KIP. On Fri, Nov 3, 2017 at 8:01 PM, Ted Yu wrote: > Looks good overall. > > bq. the creation within StreamsPartitionAssignor > > Typo above: should be StreamPartitionAssignor > > On Fri, Nov 3, 2017 at 4:49 PM, Guozhang Wang

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-04 Thread Matthias J. Sax
I like this KIP. Can you also link to https://issues.apache.org/jira/browse/KAFKA-6126 in the KIP? What I am wondering though: if we start to partially (ie, step by step) replace the existing StreamsKafkaClient with Java AdminClient, don't we need more KIPs? For example, if we use purge-api for

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-03 Thread Ted Yu
Looks good overall. bq. the creation within StreamsPartitionAssignor Typo above: should be StreamPartitionAssignor On Fri, Nov 3, 2017 at 4:49 PM, Guozhang Wang wrote: > Hello folks, > > I have filed a new KIP on adding AdminClient into Streams for internal > topic

Re: [DISCUSS] KIP-220: Add AdminClient into Kafka Streams' ClientSupplier

2017-11-03 Thread Matt Farmer
This seems like an A+ improvement to me. On Fri, Nov 3, 2017 at 7:49 PM Guozhang Wang wrote: > Hello folks, > > I have filed a new KIP on adding AdminClient into Streams for internal > topic management. > > Looking for feedback on > > * >