Thanks for the comments Colin. My only concern is that this KIP is addressing a good feature and having that extended to RoundRobinPartitioner means 1 less KIP in the future.
Would it be appropriate to extend the support to RoundRobinPartitioner too? Thanks, On Tue, 9 Jul 2019 at 17:24, Colin McCabe <cmcc...@apache.org> wrote: > Hi M, > > The RoundRobinPartitioner added by KIP-369 doesn't interact with this > KIP. If you configure your producer to use RoundRobinPartitioner, then the > DefaultPartitioner will not be used. And the "sticky" behavior is > implemented only in the DefaultPartitioner. > > regards, > Colin > > > On Tue, Jul 9, 2019, at 05:12, M. Manna wrote: > > Hello Justine, > > > > I have one item I wanted to discuss. > > > > We are currently in review stage for KAFKA-3333 where we can choose > always > > RoundRobin regardless of null/usable key. > > > > If I understood this KIP motivation correctly, you are still honouring > how > > the hashing of key works for DefaultPartitioner. Would you say that > having > > an always "Round-Robin" partitioning with "Sticky" assignment (efficient > > batching of records for a partition) doesn't deviate from your original > > intention? > > > > Thanks, > > > > On Tue, 9 Jul 2019 at 01:00, Justine Olshan <jols...@confluent.io> > wrote: > > > > > Hello all, > > > > > > If there are no more comments or concerns, I would like to start the > vote > > > on this tomorrow afternoon. > > > > > > However, if there are still topics to discuss, feel free to bring them > up > > > now. > > > > > > Thank you, > > > Justine > > > > > > On Tue, Jul 2, 2019 at 4:25 PM Justine Olshan <jols...@confluent.io> > > > wrote: > > > > > > > Hello again, > > > > > > > > Another update to the interface has been made to the KIP. > > > > Please let me know if you have any feedback! > > > > > > > > Thank you, > > > > Justine > > > > > > > > On Fri, Jun 28, 2019 at 2:52 PM Justine Olshan <jols...@confluent.io > > > > > > wrote: > > > > > > > >> Hi all, > > > >> I made some changes to the KIP. > > > >> The idea is to clean up the code, make behavior more explicit, > provide > > > >> more flexibility, and to keep default behavior the same. > > > >> > > > >> Now we will change the partition in onNewBatch, and specify the > > > >> conditions for this function call (non-keyed values, no explicit > > > >> partitions) in willCallOnNewBatch. > > > >> This clears up some of the issues with the implementation. I'm > happy to > > > >> hear further opinions and discuss this change! > > > >> > > > >> Thank you, > > > >> Justine > > > >> > > > >> On Thu, Jun 27, 2019 at 2:53 PM Colin McCabe <cmcc...@apache.org> > > > wrote: > > > >> > > > >>> On Thu, Jun 27, 2019, at 01:31, Ismael Juma wrote: > > > >>> > Thanks for the KIP Justine. It looks pretty good. A few comments: > > > >>> > > > > >>> > 1. Should we favor partitions that are not under replicated? > This is > > > >>> > something that Netflix did too. > > > >>> > > > >>> This seems like it could lead to cascading failures, right? If a > > > >>> partition becomes under-replicated because there is too much > traffic, > > > the > > > >>> producer stops sending to it, which puts even more load on the > > > remaining > > > >>> partitions, which are even more likely to fail then, etc. It also > will > > > >>> create unbalanced load patterns on the consumers. > > > >>> > > > >>> > > > > >>> > 2. If there's no measurable performance difference, I agree with > > > >>> Stanislav > > > >>> > that Optional would be better than Integer. > > > >>> > > > > >>> > 3. We should include the javadoc for the newly introduced method > that > > > >>> > specifies it and its parameters. In particular, it would good to > > > >>> specify if > > > >>> > it gets called when an explicit partition id has been provided. > > > >>> > > > >>> Agreed. > > > >>> > > > >>> best, > > > >>> Colin > > > >>> > > > >>> > > > > >>> > Ismael > > > >>> > > > > >>> > On Mon, Jun 24, 2019, 2:04 PM Justine Olshan < > jols...@confluent.io> > > > >>> wrote: > > > >>> > > > > >>> > > Hello, > > > >>> > > This is the discussion thread for KIP-480: Sticky Partitioner. > > > >>> > > > > > >>> > > > > > >>> > > > > > >>> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-480%3A+Sticky+Partitioner > > > >>> > > > > > >>> > > Thank you, > > > >>> > > Justine Olshan > > > >>> > > > > > >>> > > > > >>> > > > >> > > > > > >
