You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by François Rosière <fr...@gmail.com> on 2022/05/18 20:11:15 UTC

[DISCUSS] KIP-839: Provide builders for KafkaProducer/KafkaConsumer and KafkaStreams

Hi all,

KIP to create builders for

   - KafkaProducer
   - KafkaConsumer
   - KafkaStreams


This KIP can be seen as the continuity of the KIP-832.

KIP details:  <goog_1519423886>
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211884640
Jira issue: https://issues.apache.org/jira/browse/KAFKA-13913

Kr,

F.

Re: [DISCUSS] KIP-839: Provide builders for KafkaProducer/KafkaConsumer and KafkaStreams

Posted by Chris Egerton <fe...@gmail.com>.
Hi François,

Thanks for writing this up! A few thoughts now that I've had some time to
think this over:

1. The KIP outlines methods for the KafkaStreamsBuilder class to add
producer interceptors and consumer interceptors. Since Streams instantiates
many producers and consumers, we should probably leave these methods out of
the builder due to the concerns noted in the KIP about shared resources
being closed when a client is shut down. The existing KafkaClientSupplier
interface should be sufficient to cover the case of interceptors already,
but if not, we can explore alternatives that don't risk conflict over
shared resources.

2. In a similar vein, the KIP also outlines a method for the
KafkaStreamsBuilder class to add metrics reporters. Unlike interceptors, I
think there's still some value to this method, since a KafkaStreams
instance does instantiate its own set of metrics reporters. However, we
should probably be careful to note in the docs for this method (and in the
KIP itself) that this method only controls the metrics reporters for
Streams-specific metrics, and not for the Kafka clients that Streams uses.
We could also point people towards the KafkaClientSupplier interface if
they'd like to add metrics reporters for these Kafka clients.

3. We should probably add a builder for the Admin interface as well, since
it's also a Kafka client class and comes with the same "metric.reporters"
property (which takes a list of classes) that the other two come with.

4. Can you specify the exact package(s) that the new builders will be added
to, and the exact type signatures for their constructors and methods? You
can see KIP-585 [1] for a good example of how KIPs usually define new
classes/interfaces. You don't need to add Javadocs to everything, but if
there's something worth mentioning like the behavior proposed in item 2,
it's nice to include in a Javadoc for the relevant class/method.

5. Just for aesthetics, I wonder if we can add static builder() methods to
the KafkaProducer, KafkaConsumer, KafkaStreams, and Admin classes/interface
that return a new builder? Seems cleaner to write
"KafkaProducer.builder(props)" than "new KafkaProducerBuilder(props)". Not
a huge thing, though.

[1] -
https://cwiki.apache.org/confluence/display/KAFKA/KIP-585%3A+Filter+and+Conditional+SMTs#KIP585:FilterandConditionalSMTs-PublicInterfaces

Cheers,

Chris

On Thu, May 19, 2022 at 4:59 PM François Rosière <fr...@gmail.com>
wrote:

> @Kirk,
>
> 1. No defaults are expected. Serializers/deserializers would need to either
> by provided using the config or the builder.
> 2. As some properties would need to be closed, maybe the build method
> should only be called one time. To see if we really need to add a check for
> that.
>
> Kr,
>
> François.
>
> Le jeu. 19 mai 2022 à 00:02, Kirk True <ki...@kirktrue.pro> a écrit :
>
> > Hi François,
> >
> > Thanks for the KIP!
> >
> > A couple of questions:
> >
> >  1. Do the builders have defaults for the serializers/deserializers?
> >  2. Can the build method be called more than once on a given builder?
> >
> > Thanks,
> > Kirk
> >
> > On Wed, May 18, 2022, at 10:11 AM, François Rosière wrote:
> > > Hi all,
> > >
> > > KIP to create builders for
> > >
> > >    - KafkaProducer
> > >    - KafkaConsumer
> > >    - KafkaStreams
> > >
> > >
> > > This KIP can be seen as the continuity of the KIP-832.
> > >
> > > KIP details:  <goog_1519423886>
> > >
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211884640
> > > Jira issue: https://issues.apache.org/jira/browse/KAFKA-13913
> > >
> > > Kr,
> > >
> > > F.
> > >
> >
>

Re: [DISCUSS] KIP-839: Provide builders for KafkaProducer/KafkaConsumer and KafkaStreams

Posted by François Rosière <fr...@gmail.com>.
@Kirk,

1. No defaults are expected. Serializers/deserializers would need to either
by provided using the config or the builder.
2. As some properties would need to be closed, maybe the build method
should only be called one time. To see if we really need to add a check for
that.

Kr,

François.

Le jeu. 19 mai 2022 à 00:02, Kirk True <ki...@kirktrue.pro> a écrit :

> Hi François,
>
> Thanks for the KIP!
>
> A couple of questions:
>
>  1. Do the builders have defaults for the serializers/deserializers?
>  2. Can the build method be called more than once on a given builder?
>
> Thanks,
> Kirk
>
> On Wed, May 18, 2022, at 10:11 AM, François Rosière wrote:
> > Hi all,
> >
> > KIP to create builders for
> >
> >    - KafkaProducer
> >    - KafkaConsumer
> >    - KafkaStreams
> >
> >
> > This KIP can be seen as the continuity of the KIP-832.
> >
> > KIP details:  <goog_1519423886>
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211884640
> > Jira issue: https://issues.apache.org/jira/browse/KAFKA-13913
> >
> > Kr,
> >
> > F.
> >
>

Re: [DISCUSS] KIP-839: Provide builders for KafkaProducer/KafkaConsumer and KafkaStreams

Posted by Kirk True <ki...@kirktrue.pro>.
Hi François,

Thanks for the KIP!

A couple of questions:

 1. Do the builders have defaults for the serializers/deserializers?
 2. Can the build method be called more than once on a given builder? 

Thanks,
Kirk

On Wed, May 18, 2022, at 10:11 AM, François Rosière wrote:
> Hi all,
> 
> KIP to create builders for
> 
>    - KafkaProducer
>    - KafkaConsumer
>    - KafkaStreams
> 
> 
> This KIP can be seen as the continuity of the KIP-832.
> 
> KIP details:  <goog_1519423886>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211884640
> Jira issue: https://issues.apache.org/jira/browse/KAFKA-13913
> 
> Kr,
> 
> F.
>