Awesome, thanks! +1 from me as well.
On Thu, May 5, 2022 at 5:50 PM Artem Livshits <alivsh...@confluent.io.invalid> wrote: > Hi Guozhang, > > The current DefaultStreamPartitioner behavior is not changed by this KIP, > so I think we can track it separately. I've created a ticket: > https://issues.apache.org/jira/browse/KAFKA-13880. > > -Artem > > > > On Thu, May 5, 2022 at 10:24 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > Hello Artem, > > > > Thanks for proposing this KIP. I took a look at the current PR and also > > thought about its implications on Kafka Streams. Here are some thoughts: > > > > Today Kafka Streams use an explicit Partitioner --- note it is not > > implementing the Producer's Partitioner --- to determine the partition > > number before calling `producer.send`. Also the record is serialized > > outside the `send` call as well. That means, the record sent to the > > producer is always in `<byte[], byte[]>` type and partition id is always > > specified. > > > > The reason for serializing outside the producer is that the same producer > > is used to send to various topics with different schema, and hence we > > cannot specify a single serializer config. And the reason to determine > the > > partition id outside the producer is that we have different logic for > > windowed record v.s. non-windowed record and hence cannot have a single > > partitioner config. > > > > For the windowed partitioner it does not matter since the key would > always > > be specified and hence we would not leverage sticky partitioning. For the > > non-windowed partitioner it's possible that the key is null, and inside > the > > non-windowed customized partitioner, the `DefaultPartitioner` is used > > indeed. But with this KIP as the new logic is not encoded in the > configured > > partitioner it means Kafka Streams would not be able to leverage its > > benefits. > > > > I think we can modify the non-windowed partitioner such that when the key > > is null, we just set the partition to null, then inside the KafkaProducer > > we could still leverage on the new sticky behavior. Since in Kafka > Streams > > only sink topics data may have null keys which would not be required > state > > metadata, relaxing this in the StreamsPartitioner should be fine. > > > > > > Guozhang > > > > > > On Sat, Mar 26, 2022 at 4:05 PM Lucas Bradstreet > > <lu...@confluent.io.invalid> > > wrote: > > > > > Hi Artem, > > > > > > Thank you for all the work on this. I think it'll be quite impactful. > > > > > > +1 non-binding from me. > > > > > > Lucas > > > > > > On Wed, Mar 23, 2022 at 8:27 PM Luke Chen <show...@gmail.com> wrote: > > > > > > > Hi Artem, > > > > > > > > Thanks for the KIP and the patience during discussion! > > > > +1 binding from me. > > > > > > > > Luke > > > > > > > > On Thu, Mar 24, 2022 at 3:43 AM Ismael Juma <ism...@juma.me.uk> > wrote: > > > > > > > > > Thanks for the KIP and for taking the time to address all the > > feedback. > > > > +1 > > > > > (binding) > > > > > > > > > > Ismael > > > > > > > > > > On Mon, Mar 21, 2022 at 4:52 PM Artem Livshits > > > > > <alivsh...@confluent.io.invalid> wrote: > > > > > > > > > > > Hi all, > > > > > > > > > > > > I'd like to start a vote on > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-794%3A+Strictly+Uniform+Sticky+Partitioner > > > > > > . > > > > > > > > > > > > -Artem > > > > > > > > > > > > > > > > > > > > > > > > -- > > -- Guozhang > > > -- -- Guozhang
