You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jack Tomy <ja...@gmail.com> on 2023/08/03 12:43:04 UTC
Re: [DISCUSS] KIP-953: partition method to be overloaded to accept headers as well.
Hey Andrew, Sagar
Please share your thoughts. Thanks.
On Mon, Jul 31, 2023 at 5:58 PM Jack Tomy <ja...@gmail.com> wrote:
> Hey Andrew, Sagar
>
> Thanks. I'm travelling so sorry for being brief and getting back late.
>
> 1. For the first concern, that is moving in a direction of server side
> partitioner, the idea seems very much promising but I believe we still have
> a long way to go. Since the proposal/design for the same is still not
> available, it's hard for me to defend my proposal.
> 2. For the second concern:
> 2.1 Loss of order in messages, I believe the ordering of messages is
> never promised and the partitioner has no requirement to ensure the same.
> It is upto the user to implement/use a partitioner which ensures ordering
> based on keys.
> 2.2 Key deciding the partitioner, It is totally up to the user to decide
> the partition regardless of the key, we are also passing the value to the
> partitioner. Even the existing implementation receives the value which lets
> the user decide the partition based on value.
> 2.3 Sending to a specific partition, for this, I need to be aware of the
> total number of partitions, but if I can do that same in partitioner, the
> cluster param gives me all the information I want.
>
> I would also quote a line from KIP-82 where headers were added to the
> serializer : The payload is traditionally for the business object, and *headers
> are traditionally used for transport routing*, filtering etc. So I
> believe when a user wants to add some routing information (in this case
> which set of partitions to go for), headers seem to be the right place.
>
>
>
> On Sat, Jul 29, 2023 at 8:48 PM Sagar <sa...@gmail.com> wrote:
>
>> Hi Andrew,
>>
>> Thanks for your comments.
>>
>> 1) Yes that makes sense and that's what even would expect to see as well.
>> I
>> just wanted to highlight that we might still need a way to let client side
>> partitioning logic be present as well. Anyways, I am good on this point.
>> 2) The example provided does seem achievable by simply attaching the
>> partition number in the ProducerRecord. I guess if we can't find any
>> further examples which strengthen the case of this partitioner, it might
>> be
>> harder to justify adding it.
>>
>>
>> Thanks!
>> Sagar.
>>
>> On Fri, Jul 28, 2023 at 2:05 PM Andrew Schofield <
>> andrew_schofield_jira@outlook.com> wrote:
>>
>> > Hi Sagar,
>> > Thanks for your comments.
>> >
>> > 1) Server-side partitioning doesn’t necessarily mean that there’s only
>> one
>> > way to do it. It just means that the partitioning logic runs on the
>> broker
>> > and
>> > any configuration of partitioning applies to the broker’s partitioner.
>> If
>> > we ever
>> > see a KIP for this, that’s the kind of thing I would expect to see.
>> >
>> > 2) In the priority example in the KIP, there is a kind of contract
>> between
>> > the
>> > producers and consumers so that some records can be processed before
>> > others regardless of the order in which they were sent. The producer
>> > wants to apply special significance to a particular header to control
>> which
>> > partition is used. I would simply achieve this by setting the partition
>> > number
>> > in the ProducerRecord at the time of sending.
>> >
>> > I don’t think the KIP proposes adjusting the built-in partitioner or
>> > adding to AK
>> > a new one that uses headers in the partitioning decision. So, any
>> > configuration
>> > for a partitioner that does support headers would be up to the
>> > implementation
>> > of that specific partitioner. Partitioner implements Configurable.
>> >
>> > I’m just providing an alternative view and I’m not particularly opposed
>> to
>> > the KIP.
>> > I just don’t think it quite merits the work involved to get it voted and
>> > merged.
>> > As an aside, a long time ago, I created a small KIP that was never
>> adopted
>> > and I didn’t push it because I eventually didn’t need it.
>> >
>> > Thanks,
>> > Andrew
>> >
>> > > On 28 Jul 2023, at 05:15, Sagar <sa...@gmail.com> wrote:
>> > >
>> > > Hey Andrew,
>> > >
>> > > Thanks for the review. Since I had reviewed the KIP I thought I would
>> > also
>> > > respond. Of course Jack has the final say on this since he wrote the
>> KIP.
>> > >
>> > > 1) This is an interesting point and I hadn't considered it. The
>> > > comparison with KIP-848 is a valid one but even within that KIP, it
>> > allows
>> > > client side partitioning for power users like Streams. So while we
>> would
>> > > want to move away from client side partitioner as much as possible, we
>> > > still shouldn't do away completely with Client side partitioning and
>> end
>> > up
>> > > being in a state of inflexibility for different kinds of usecases.
>> This
>> > is
>> > > my opinion though and you have more context on Clients, so would like
>> to
>> > > know your thoughts on this.
>> > >
>> > > 2) Regarding this, I assumed that since the headers are already part
>> of
>> > the
>> > > consumer records they should have access to the headers and if there
>> is a
>> > > contract b/w the applications producing and the application consuming,
>> > that
>> > > decisioning should be transparent. Was my assumption incorrect? But as
>> > you
>> > > rightly pointed out header based partitioning with keys is going to
>> lead
>> > to
>> > > surprising results. Assuming there is merit in this proposal, do you
>> > think
>> > > we should ignore the keys in this case (similar to the effect of
>> > > setting *partitioner.ignore.keys
>> > > *config to false) and document it appropriately?
>> > >
>> > > Let me know what you think.
>> > >
>> > > Thanks!
>> > > Sagar.
>> > >
>> > >
>> > > On Thu, Jul 27, 2023 at 9:41 PM Andrew Schofield <
>> > > andrew_schofield_jira@outlook.com> wrote:
>> > >
>> > >> Hi Jack,
>> > >> Thanks for the KIP. I have a few concerns about the idea.
>> > >>
>> > >> 1) I think that while a client-side partitioner seems like a neat
>> idea
>> > and
>> > >> it’s an established part of Kafka,
>> > >> it’s one of the things which makes Kafka clients quite complicated.
>> Just
>> > >> as KIP-848 is moving from
>> > >> client-side assignors to server-side assignors, I wonder whether
>> really
>> > we
>> > >> should be looking to make
>> > >> partitioning a server-side capability too over time. So, I’m not
>> > convinced
>> > >> that making the Partitioner
>> > >> interface richer is moving in the right direction.
>> > >>
>> > >> 2) For records with a key, the partitioner usually calculates the
>> > >> partition from the key. This means
>> > >> that records with the same key end up on the same partition. Many
>> > >> applications expect this to give ordering.
>> > >> Log compaction expects this. There are situations in which records
>> have
>> > to
>> > >> be repartitioned, such as
>> > >> sometimes happens with Kafka Streams. I think that a header-based
>> > >> partitioner for records which have
>> > >> keys is going to be surprising and only going to have limited
>> > >> applicability as a result.
>> > >>
>> > >> The tricky part about clever partitioning is that downstream systems
>> > have
>> > >> no idea how the partition
>> > >> number was arrived at, so they do not truly understand how the
>> ordering
>> > >> was derived. I do think that
>> > >> perhaps there’s value to being able to influence the partitioning
>> using
>> > >> the headers, but I wonder if actually
>> > >> transforming the headers into an “ordering context” that then flows
>> with
>> > >> the record as it moves through
>> > >> the system would be a stronger solution. Today, the key is the
>> ordering
>> > >> context. Maybe it should be a
>> > >> concept in its own right and the Producer could configure a converter
>> > from
>> > >> headers to ordering context.
>> > >> That is of course a much bigger change.
>> > >>
>> > >> In one of the examples you mention in the KIP, you mentioned using a
>> > >> header to control priority. I guess the
>> > >> idea is to preferentially process records off specific partitions so
>> > they
>> > >> can overtake lower priority records.
>> > >> I suggest just sending the records explicitly to specific partitions
>> to
>> > >> achieve this.
>> > >>
>> > >> Sorry for the essay, but you did ask for people to share their
>> thoughts
>> > :)
>> > >>
>> > >> Just my opinion. Let’s see what others think.
>> > >>
>> > >> Thanks,
>> > >> Andrew
>> > >>
>> > >>> On 25 Jul 2023, at 14:58, Jack Tomy <ja...@gmail.com> wrote:
>> > >>>
>> > >>> Hey @Sagar
>> > >>>
>> > >>> Thanks again for the review.
>> > >>> 1. "a null headers value is equivalent to invoking the older
>> partition
>> > >>> method", this is not true. If someone makes an implementation and
>> the
>> > >>> headers come as null, still the new implementation will take effect.
>> > >>> Instead I have added : "Not overriding this method in the
>> Partitioner
>> > >>> interface has the same behaviour as using the existing method."
>> > >>> 2. Corrected.
>> > >>>
>> > >>> Hey @Sagar and everyone,
>> > >>> Please have a look at the new version and share your thoughts.
>> > >>>
>> > >>
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> > >>> Like Sagar mentioned, I would also request more people who have more
>> > >>> context on clients to chime in.
>> > >>>
>> > >>>
>> > >>> On Tue, Jul 25, 2023 at 2:58 PM Sagar <sa...@gmail.com>
>> > wrote:
>> > >>>
>> > >>>> Hi Jack,
>> > >>>>
>> > >>>> Thanks I have a couple of final comments and then I am good.
>> > >>>>
>> > >>>> 1) Can you elaborate on the Javadocs of the partition headers
>> argument
>> > >> to
>> > >>>> specify that a null headers value is equivalent to invoking the
>> older
>> > >>>> partition method? It is apparent but generally good to call out.
>> > >>>> 2) In the Compatibility section, you have mentioned backward
>> > >> comparable. I
>> > >>>> believe it should be *backward compatible change.*
>> > >>>>
>> > >>>> I don't have other comments. Post this, probably someone else who
>> has
>> > >> more
>> > >>>> context on Clients can also chime in on this before we can move
>> this
>> > to
>> > >>>> Voting.
>> > >>>>
>> > >>>> Thanks!
>> > >>>> Sagar.
>> > >>>>
>> > >>>>
>> > >>>> On Sat, Jul 22, 2023 at 10:09 AM Jack Tomy <ja...@gmail.com>
>> > >> wrote:
>> > >>>>
>> > >>>>> Hey @Sagar,
>> > >>>>>
>> > >>>>> Thank you again for the response and feedback.
>> > >>>>>
>> > >>>>> 1. Though the ask wasn't very clear to me I have attached the
>> > Javadoc
>> > >>>> as
>> > >>>>> per your suggestion. Please have a look and let me know if this
>> > meets
>> > >>>>> the
>> > >>>>> expectations.
>> > >>>>> 2. Done.
>> > >>>>> 3. Done
>> > >>>>> 4. Done
>> > >>>>>
>> > >>>>> Hey @Sagar and everyone,
>> > >>>>> Please have a look at the new version and share your thoughts.
>> > >>>>>
>> > >>>>
>> > >>
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> > >>>>>
>> > >>>>> On Thu, Jul 20, 2023 at 9:46 PM Sagar <sa...@gmail.com>
>> > >> wrote:
>> > >>>>>
>> > >>>>>> Thanks Jack for the updates.
>> > >>>>>>
>> > >>>>>> Some more feedback:
>> > >>>>>>
>> > >>>>>> 1) It would be better if you can add the Javadoc in the Public
>> > >>>> interfaces
>> > >>>>>> section. That is a general practice used which gives the readers
>> of
>> > >> the
>> > >>>>> KIP
>> > >>>>>> a high level idea of the Public Interfaces.
>> > >>>>>>
>> > >>>>>> 2) In the proposed section, the bit about marking headers as read
>> > only
>> > >>>>>> seems like an implementation detail This can generally be
>> avoided in
>> > >>>>> KIPs.
>> > >>>>>>
>> > >>>>>> 3) Also, in the Deprecation section, can you mention again that
>> this
>> > >>>> is a
>> > >>>>>> backward compatible change and the reason for it (already done in
>> > the
>> > >>>>>> Proposed Changes section).
>> > >>>>>>
>> > >>>>>> 4) In the Testing Plan section, there is still the KIP template
>> bit
>> > >>>>> copied
>> > >>>>>> over. That can be removed.
>> > >>>>>>
>> > >>>>>> Thanks!
>> > >>>>>> Sagar.
>> > >>>>>>
>> > >>>>>>
>> > >>>>>> On Thu, Jul 20, 2023 at 2:48 PM Jack Tomy <jacktomy107@gmail.com
>> >
>> > >>>> wrote:
>> > >>>>>>
>> > >>>>>>> Hey Everyone,
>> > >>>>>>>
>> > >>>>>>> Please consider this as a reminder and share your feedback.
>> Thank
>> > >>>> you.
>> > >>>>>>>
>> > >>>>>>
>> > >>>>>
>> > >>>>
>> > >>
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> > >>>>>>>
>> > >>>>>>> On Tue, Jul 18, 2023 at 5:43 PM Jack Tomy <
>> jacktomy107@gmail.com>
>> > >>>>> wrote:
>> > >>>>>>>
>> > >>>>>>>> Hey @Sagar,
>> > >>>>>>>>
>> > >>>>>>>> Thank you for the response and feedback.
>> > >>>>>>>>
>> > >>>>>>>> 1. Done
>> > >>>>>>>> 2. Yeah, that was a mistake from my end. Corrected.
>> > >>>>>>>> 3. Can you please elaborate this, I have added the java doc
>> > >>>> along
>> > >>>>>> with
>> > >>>>>>>> the code changes. Should I paste the same in KIP too?
>> > >>>>>>>> 4. Moved.
>> > >>>>>>>> 5. I have added one more use case, it is actually helpful in
>> any
>> > >>>>>>>> situation where you want to pass some information to partition
>> > >>>>>> method
>> > >>>>>>> but
>> > >>>>>>>> don't have to have it in the key or value.
>> > >>>>>>>> 6. Added.
>> > >>>>>>>>
>> > >>>>>>>>
>> > >>>>>>>> Hey @Sagar and everyone,
>> > >>>>>>>> Please have a look at the new version and share your thoughts.
>> > >>>>>>>>
>> > >>>>>>>
>> > >>>>>>
>> > >>>>>
>> > >>>>
>> > >>
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> > >>>>>>>>
>> > >>>>>>>>
>> > >>>>>>>> On Tue, Jul 18, 2023 at 9:53 AM Sagar <
>> sagarmeansocean@gmail.com>
>> > >>>>>> wrote:
>> > >>>>>>>>
>> > >>>>>>>>> Hi Jack,
>> > >>>>>>>>>
>> > >>>>>>>>> Thanks for the KIP! Seems like an interesting idea. I have
>> some
>> > >>>>>>> feedback:
>> > >>>>>>>>>
>> > >>>>>>>>> 1) It would be great if you could clean up the text that
>> seems to
>> > >>>>>> mimic
>> > >>>>>>>>> the
>> > >>>>>>>>> KIP template. It is generally not required in the KIP.
>> > >>>>>>>>>
>> > >>>>>>>>> 2) In the Public Interfaces where you mentioned *Partitioner
>> > >>>> method
>> > >>>>> in
>> > >>>>>>>>> **org/apache/kafka/clients/producer
>> > >>>>>>>>> will have the following update*, I believe you meant the
>> > >>>> Partitioner
>> > >>>>>>>>> *interface*?
>> > >>>>>>>>>
>> > >>>>>>>>> 3) Staying on Public Interface, it is generally preferable to
>> add
>> > >>>> a
>> > >>>>>>>>> Javadocs section along with the newly added method. You could
>> > also
>> > >>>>>>>>> describe
>> > >>>>>>>>> the behaviour of it invoking the default existing method.
>> > >>>>>>>>>
>> > >>>>>>>>> 4) The option that is mentioned in the Rejected Alternatives,
>> > >>>> seems
>> > >>>>>> more
>> > >>>>>>>>> like a workaround to the current problem that you are
>> describing.
>> > >>>>> That
>> > >>>>>>>>> could be added to the Motivation section IMO.
>> > >>>>>>>>>
>> > >>>>>>>>> 5) Can you also add some more examples of scenarios where this
>> > >>>> would
>> > >>>>>> be
>> > >>>>>>>>> helpful? The only scenario mentioned seems to have a
>> workaround.
>> > >>>>> Just
>> > >>>>>>>>> trying to ensure that we have a strong enough motivation
>> before
>> > >>>>>> adding a
>> > >>>>>>>>> public API.
>> > >>>>>>>>>
>> > >>>>>>>>> 6) One thing which should also be worth noting down would be
>> what
>> > >>>>>>> happens
>> > >>>>>>>>> if users override both methods, only one method (new or old)
>> and
>> > >>>> no
>> > >>>>>>>>> methods
>> > >>>>>>>>> (the default behaviour). It would help in understanding the
>> > >>>> proposal
>> > >>>>>>>>> better.
>> > >>>>>>>>>
>> > >>>>>>>>> Thanks!
>> > >>>>>>>>> Sagar.
>> > >>>>>>>>>
>> > >>>>>>>>>
>> > >>>>>>>>> On Mon, Jul 17, 2023 at 9:19 PM Jack Tomy <
>> jacktomy107@gmail.com
>> > >
>> > >>>>>>> wrote:
>> > >>>>>>>>>
>> > >>>>>>>>>> Hey everyone,
>> > >>>>>>>>>>
>> > >>>>>>>>>> Not seeing much discussion on the KPI. Might be because it is
>> > >>>> too
>> > >>>>>>>>>> obvious 😉.
>> > >>>>>>>>>>
>> > >>>>>>>>>> If there are no more comments, I will start the VOTE in the
>> > >>>> coming
>> > >>>>>>> days.
>> > >>>>>>>>>>
>> > >>>>>>>>>> On Sat, Jul 15, 2023 at 8:48 PM Jack Tomy <
>> > >>>> jacktomy107@gmail.com>
>> > >>>>>>>>> wrote:
>> > >>>>>>>>>>
>> > >>>>>>>>>>> Hey everyone,
>> > >>>>>>>>>>>
>> > >>>>>>>>>>> Please take a look at the KPI below and provide your
>> > >>>> suggestions
>> > >>>>>> and
>> > >>>>>>>>>>> feedback. TIA.
>> > >>>>>>>>>>>
>> > >>>>>>>>>>>
>> > >>>>>>>>>>
>> > >>>>>>>>>
>> > >>>>>>>
>> > >>>>>>
>> > >>>>>
>> > >>>>
>> > >>
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> > >>>>>>>>>>>
>> > >>>>>>>>>>>
>> > >>>>>>>>>>> --
>> > >>>>>>>>>>> Best Regards
>> > >>>>>>>>>>> *Jack*
>> > >>>>>>>>>>>
>> > >>>>>>>>>>
>> > >>>>>>>>>>
>> > >>>>>>>>>> --
>> > >>>>>>>>>> Best Regards
>> > >>>>>>>>>> *Jack*
>> > >>>>>>>>>>
>> > >>>>>>>>>
>> > >>>>>>>>
>> > >>>>>>>>
>> > >>>>>>>> --
>> > >>>>>>>> Best Regards
>> > >>>>>>>> *Jack*
>> > >>>>>>>>
>> > >>>>>>>
>> > >>>>>>>
>> > >>>>>>> --
>> > >>>>>>> Best Regards
>> > >>>>>>> *Jack*
>> > >>>>>>>
>> > >>>>>>
>> > >>>>>
>> > >>>>>
>> > >>>>> --
>> > >>>>> Best Regards
>> > >>>>> *Jack*
>> > >>>>>
>> > >>>>
>> > >>>
>> > >>>
>> > >>> --
>> > >>> Best Regards
>> > >>> *Jack*
>> > >>
>> > >>
>> >
>> >
>>
>
>
> --
> Best Regards
> *Jack*
>
--
Best Regards
*Jack*
Re: [DISCUSS] KIP-953: partition method to be overloaded to accept headers as well.
Posted by Jack Tomy <ja...@gmail.com>.
Hey everyone
I'm closing the discussion as I haven't heard from 3 days.
Before I close the thread I would like to thank Sagar and Andrew for their
suggestions and feedback. I believe it has helped to improve the KIP.
Thank you all.
On Fri, Aug 4, 2023 at 10:26 AM Jack Tomy <ja...@gmail.com> wrote:
> All right, Thanks Andrew.
>
> Hey everyone,
> Please share your thoughts and feedback on the KIP :
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>
>
>
>
> On Fri, Aug 4, 2023 at 2:50 AM Andrew Schofield <
> andrew_schofield_jira@outlook.com> wrote:
>
>> Hi Jack,
>> I do understand the idea of extending the Partitioner interface so that
>> people are now able to use headers in the partitioning decision, and I see
>> that it’s filling in gap in the interface which was left when headers were
>> originally added.
>>
>> Experience with non-default partitioning schemes in the past makes me
>> unlikely to use anything other than the default partitioning scheme.
>> But I wouldn’t let that dissuade you.
>>
>> Thanks,
>> Andrew
>>
>> > On 3 Aug 2023, at 13:43, Jack Tomy <ja...@gmail.com> wrote:
>> >
>> > Hey Andrew, Sagar
>> >
>> > Please share your thoughts. Thanks.
>> >
>> >
>> >
>> > On Mon, Jul 31, 2023 at 5:58 PM Jack Tomy <ja...@gmail.com>
>> wrote:
>> >
>> >> Hey Andrew, Sagar
>> >>
>> >> Thanks. I'm travelling so sorry for being brief and getting back late.
>> >>
>> >> 1. For the first concern, that is moving in a direction of server side
>> >> partitioner, the idea seems very much promising but I believe we still
>> have
>> >> a long way to go. Since the proposal/design for the same is still not
>> >> available, it's hard for me to defend my proposal.
>> >> 2. For the second concern:
>> >> 2.1 Loss of order in messages, I believe the ordering of messages is
>> >> never promised and the partitioner has no requirement to ensure the
>> same.
>> >> It is upto the user to implement/use a partitioner which ensures
>> ordering
>> >> based on keys.
>> >> 2.2 Key deciding the partitioner, It is totally up to the user to
>> decide
>> >> the partition regardless of the key, we are also passing the value to
>> the
>> >> partitioner. Even the existing implementation receives the value which
>> lets
>> >> the user decide the partition based on value.
>> >> 2.3 Sending to a specific partition, for this, I need to be aware of
>> the
>> >> total number of partitions, but if I can do that same in partitioner,
>> the
>> >> cluster param gives me all the information I want.
>> >>
>> >> I would also quote a line from KIP-82 where headers were added to the
>> >> serializer : The payload is traditionally for the business object, and
>> *headers
>> >> are traditionally used for transport routing*, filtering etc. So I
>> >> believe when a user wants to add some routing information (in this case
>> >> which set of partitions to go for), headers seem to be the right place.
>> >>
>> >>
>> >>
>> >> On Sat, Jul 29, 2023 at 8:48 PM Sagar <sa...@gmail.com>
>> wrote:
>> >>
>> >>> Hi Andrew,
>> >>>
>> >>> Thanks for your comments.
>> >>>
>> >>> 1) Yes that makes sense and that's what even would expect to see as
>> well.
>> >>> I
>> >>> just wanted to highlight that we might still need a way to let client
>> side
>> >>> partitioning logic be present as well. Anyways, I am good on this
>> point.
>> >>> 2) The example provided does seem achievable by simply attaching the
>> >>> partition number in the ProducerRecord. I guess if we can't find any
>> >>> further examples which strengthen the case of this partitioner, it
>> might
>> >>> be
>> >>> harder to justify adding it.
>> >>>
>> >>>
>> >>> Thanks!
>> >>> Sagar.
>> >>>
>> >>> On Fri, Jul 28, 2023 at 2:05 PM Andrew Schofield <
>> >>> andrew_schofield_jira@outlook.com> wrote:
>> >>>
>> >>>> Hi Sagar,
>> >>>> Thanks for your comments.
>> >>>>
>> >>>> 1) Server-side partitioning doesn’t necessarily mean that there’s
>> only
>> >>> one
>> >>>> way to do it. It just means that the partitioning logic runs on the
>> >>> broker
>> >>>> and
>> >>>> any configuration of partitioning applies to the broker’s
>> partitioner.
>> >>> If
>> >>>> we ever
>> >>>> see a KIP for this, that’s the kind of thing I would expect to see.
>> >>>>
>> >>>> 2) In the priority example in the KIP, there is a kind of contract
>> >>> between
>> >>>> the
>> >>>> producers and consumers so that some records can be processed before
>> >>>> others regardless of the order in which they were sent. The producer
>> >>>> wants to apply special significance to a particular header to control
>> >>> which
>> >>>> partition is used. I would simply achieve this by setting the
>> partition
>> >>>> number
>> >>>> in the ProducerRecord at the time of sending.
>> >>>>
>> >>>> I don’t think the KIP proposes adjusting the built-in partitioner or
>> >>>> adding to AK
>> >>>> a new one that uses headers in the partitioning decision. So, any
>> >>>> configuration
>> >>>> for a partitioner that does support headers would be up to the
>> >>>> implementation
>> >>>> of that specific partitioner. Partitioner implements Configurable.
>> >>>>
>> >>>> I’m just providing an alternative view and I’m not particularly
>> opposed
>> >>> to
>> >>>> the KIP.
>> >>>> I just don’t think it quite merits the work involved to get it voted
>> and
>> >>>> merged.
>> >>>> As an aside, a long time ago, I created a small KIP that was never
>> >>> adopted
>> >>>> and I didn’t push it because I eventually didn’t need it.
>> >>>>
>> >>>> Thanks,
>> >>>> Andrew
>> >>>>
>> >>>>> On 28 Jul 2023, at 05:15, Sagar <sa...@gmail.com> wrote:
>> >>>>>
>> >>>>> Hey Andrew,
>> >>>>>
>> >>>>> Thanks for the review. Since I had reviewed the KIP I thought I
>> would
>> >>>> also
>> >>>>> respond. Of course Jack has the final say on this since he wrote the
>> >>> KIP.
>> >>>>>
>> >>>>> 1) This is an interesting point and I hadn't considered it. The
>> >>>>> comparison with KIP-848 is a valid one but even within that KIP, it
>> >>>> allows
>> >>>>> client side partitioning for power users like Streams. So while we
>> >>> would
>> >>>>> want to move away from client side partitioner as much as possible,
>> we
>> >>>>> still shouldn't do away completely with Client side partitioning and
>> >>> end
>> >>>> up
>> >>>>> being in a state of inflexibility for different kinds of usecases.
>> >>> This
>> >>>> is
>> >>>>> my opinion though and you have more context on Clients, so would
>> like
>> >>> to
>> >>>>> know your thoughts on this.
>> >>>>>
>> >>>>> 2) Regarding this, I assumed that since the headers are already part
>> >>> of
>> >>>> the
>> >>>>> consumer records they should have access to the headers and if there
>> >>> is a
>> >>>>> contract b/w the applications producing and the application
>> consuming,
>> >>>> that
>> >>>>> decisioning should be transparent. Was my assumption incorrect? But
>> as
>> >>>> you
>> >>>>> rightly pointed out header based partitioning with keys is going to
>> >>> lead
>> >>>> to
>> >>>>> surprising results. Assuming there is merit in this proposal, do you
>> >>>> think
>> >>>>> we should ignore the keys in this case (similar to the effect of
>> >>>>> setting *partitioner.ignore.keys
>> >>>>> *config to false) and document it appropriately?
>> >>>>>
>> >>>>> Let me know what you think.
>> >>>>>
>> >>>>> Thanks!
>> >>>>> Sagar.
>> >>>>>
>> >>>>>
>> >>>>> On Thu, Jul 27, 2023 at 9:41 PM Andrew Schofield <
>> >>>>> andrew_schofield_jira@outlook.com> wrote:
>> >>>>>
>> >>>>>> Hi Jack,
>> >>>>>> Thanks for the KIP. I have a few concerns about the idea.
>> >>>>>>
>> >>>>>> 1) I think that while a client-side partitioner seems like a neat
>> >>> idea
>> >>>> and
>> >>>>>> it’s an established part of Kafka,
>> >>>>>> it’s one of the things which makes Kafka clients quite complicated.
>> >>> Just
>> >>>>>> as KIP-848 is moving from
>> >>>>>> client-side assignors to server-side assignors, I wonder whether
>> >>> really
>> >>>> we
>> >>>>>> should be looking to make
>> >>>>>> partitioning a server-side capability too over time. So, I’m not
>> >>>> convinced
>> >>>>>> that making the Partitioner
>> >>>>>> interface richer is moving in the right direction.
>> >>>>>>
>> >>>>>> 2) For records with a key, the partitioner usually calculates the
>> >>>>>> partition from the key. This means
>> >>>>>> that records with the same key end up on the same partition. Many
>> >>>>>> applications expect this to give ordering.
>> >>>>>> Log compaction expects this. There are situations in which records
>> >>> have
>> >>>> to
>> >>>>>> be repartitioned, such as
>> >>>>>> sometimes happens with Kafka Streams. I think that a header-based
>> >>>>>> partitioner for records which have
>> >>>>>> keys is going to be surprising and only going to have limited
>> >>>>>> applicability as a result.
>> >>>>>>
>> >>>>>> The tricky part about clever partitioning is that downstream
>> systems
>> >>>> have
>> >>>>>> no idea how the partition
>> >>>>>> number was arrived at, so they do not truly understand how the
>> >>> ordering
>> >>>>>> was derived. I do think that
>> >>>>>> perhaps there’s value to being able to influence the partitioning
>> >>> using
>> >>>>>> the headers, but I wonder if actually
>> >>>>>> transforming the headers into an “ordering context” that then flows
>> >>> with
>> >>>>>> the record as it moves through
>> >>>>>> the system would be a stronger solution. Today, the key is the
>> >>> ordering
>> >>>>>> context. Maybe it should be a
>> >>>>>> concept in its own right and the Producer could configure a
>> converter
>> >>>> from
>> >>>>>> headers to ordering context.
>> >>>>>> That is of course a much bigger change.
>> >>>>>>
>> >>>>>> In one of the examples you mention in the KIP, you mentioned using
>> a
>> >>>>>> header to control priority. I guess the
>> >>>>>> idea is to preferentially process records off specific partitions
>> so
>> >>>> they
>> >>>>>> can overtake lower priority records.
>> >>>>>> I suggest just sending the records explicitly to specific
>> partitions
>> >>> to
>> >>>>>> achieve this.
>> >>>>>>
>> >>>>>> Sorry for the essay, but you did ask for people to share their
>> >>> thoughts
>> >>>> :)
>> >>>>>>
>> >>>>>> Just my opinion. Let’s see what others think.
>> >>>>>>
>> >>>>>> Thanks,
>> >>>>>> Andrew
>> >>>>>>
>> >>>>>>> On 25 Jul 2023, at 14:58, Jack Tomy <ja...@gmail.com>
>> wrote:
>> >>>>>>>
>> >>>>>>> Hey @Sagar
>> >>>>>>>
>> >>>>>>> Thanks again for the review.
>> >>>>>>> 1. "a null headers value is equivalent to invoking the older
>> >>> partition
>> >>>>>>> method", this is not true. If someone makes an implementation and
>> >>> the
>> >>>>>>> headers come as null, still the new implementation will take
>> effect.
>> >>>>>>> Instead I have added : "Not overriding this method in the
>> >>> Partitioner
>> >>>>>>> interface has the same behaviour as using the existing method."
>> >>>>>>> 2. Corrected.
>> >>>>>>>
>> >>>>>>> Hey @Sagar and everyone,
>> >>>>>>> Please have a look at the new version and share your thoughts.
>> >>>>>>>
>> >>>>>>
>> >>>>
>> >>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> >>>>>>> Like Sagar mentioned, I would also request more people who have
>> more
>> >>>>>>> context on clients to chime in.
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> On Tue, Jul 25, 2023 at 2:58 PM Sagar <sa...@gmail.com>
>> >>>> wrote:
>> >>>>>>>
>> >>>>>>>> Hi Jack,
>> >>>>>>>>
>> >>>>>>>> Thanks I have a couple of final comments and then I am good.
>> >>>>>>>>
>> >>>>>>>> 1) Can you elaborate on the Javadocs of the partition headers
>> >>> argument
>> >>>>>> to
>> >>>>>>>> specify that a null headers value is equivalent to invoking the
>> >>> older
>> >>>>>>>> partition method? It is apparent but generally good to call out.
>> >>>>>>>> 2) In the Compatibility section, you have mentioned backward
>> >>>>>> comparable. I
>> >>>>>>>> believe it should be *backward compatible change.*
>> >>>>>>>>
>> >>>>>>>> I don't have other comments. Post this, probably someone else who
>> >>> has
>> >>>>>> more
>> >>>>>>>> context on Clients can also chime in on this before we can move
>> >>> this
>> >>>> to
>> >>>>>>>> Voting.
>> >>>>>>>>
>> >>>>>>>> Thanks!
>> >>>>>>>> Sagar.
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> On Sat, Jul 22, 2023 at 10:09 AM Jack Tomy <
>> jacktomy107@gmail.com>
>> >>>>>> wrote:
>> >>>>>>>>
>> >>>>>>>>> Hey @Sagar,
>> >>>>>>>>>
>> >>>>>>>>> Thank you again for the response and feedback.
>> >>>>>>>>>
>> >>>>>>>>> 1. Though the ask wasn't very clear to me I have attached the
>> >>>> Javadoc
>> >>>>>>>> as
>> >>>>>>>>> per your suggestion. Please have a look and let me know if this
>> >>>> meets
>> >>>>>>>>> the
>> >>>>>>>>> expectations.
>> >>>>>>>>> 2. Done.
>> >>>>>>>>> 3. Done
>> >>>>>>>>> 4. Done
>> >>>>>>>>>
>> >>>>>>>>> Hey @Sagar and everyone,
>> >>>>>>>>> Please have a look at the new version and share your thoughts.
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>
>> >>>>
>> >>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> >>>>>>>>>
>> >>>>>>>>> On Thu, Jul 20, 2023 at 9:46 PM Sagar <
>> sagarmeansocean@gmail.com>
>> >>>>>> wrote:
>> >>>>>>>>>
>> >>>>>>>>>> Thanks Jack for the updates.
>> >>>>>>>>>>
>> >>>>>>>>>> Some more feedback:
>> >>>>>>>>>>
>> >>>>>>>>>> 1) It would be better if you can add the Javadoc in the Public
>> >>>>>>>> interfaces
>> >>>>>>>>>> section. That is a general practice used which gives the
>> readers
>> >>> of
>> >>>>>> the
>> >>>>>>>>> KIP
>> >>>>>>>>>> a high level idea of the Public Interfaces.
>> >>>>>>>>>>
>> >>>>>>>>>> 2) In the proposed section, the bit about marking headers as
>> read
>> >>>> only
>> >>>>>>>>>> seems like an implementation detail This can generally be
>> >>> avoided in
>> >>>>>>>>> KIPs.
>> >>>>>>>>>>
>> >>>>>>>>>> 3) Also, in the Deprecation section, can you mention again that
>> >>> this
>> >>>>>>>> is a
>> >>>>>>>>>> backward compatible change and the reason for it (already done
>> in
>> >>>> the
>> >>>>>>>>>> Proposed Changes section).
>> >>>>>>>>>>
>> >>>>>>>>>> 4) In the Testing Plan section, there is still the KIP template
>> >>> bit
>> >>>>>>>>> copied
>> >>>>>>>>>> over. That can be removed.
>> >>>>>>>>>>
>> >>>>>>>>>> Thanks!
>> >>>>>>>>>> Sagar.
>> >>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>> On Thu, Jul 20, 2023 at 2:48 PM Jack Tomy <
>> jacktomy107@gmail.com
>> >>>>
>> >>>>>>>> wrote:
>> >>>>>>>>>>
>> >>>>>>>>>>> Hey Everyone,
>> >>>>>>>>>>>
>> >>>>>>>>>>> Please consider this as a reminder and share your feedback.
>> >>> Thank
>> >>>>>>>> you.
>> >>>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>
>> >>>>
>> >>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> >>>>>>>>>>>
>> >>>>>>>>>>> On Tue, Jul 18, 2023 at 5:43 PM Jack Tomy <
>> >>> jacktomy107@gmail.com>
>> >>>>>>>>> wrote:
>> >>>>>>>>>>>
>> >>>>>>>>>>>> Hey @Sagar,
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Thank you for the response and feedback.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> 1. Done
>> >>>>>>>>>>>> 2. Yeah, that was a mistake from my end. Corrected.
>> >>>>>>>>>>>> 3. Can you please elaborate this, I have added the java doc
>> >>>>>>>> along
>> >>>>>>>>>> with
>> >>>>>>>>>>>> the code changes. Should I paste the same in KIP too?
>> >>>>>>>>>>>> 4. Moved.
>> >>>>>>>>>>>> 5. I have added one more use case, it is actually helpful in
>> >>> any
>> >>>>>>>>>>>> situation where you want to pass some information to
>> partition
>> >>>>>>>>>> method
>> >>>>>>>>>>> but
>> >>>>>>>>>>>> don't have to have it in the key or value.
>> >>>>>>>>>>>> 6. Added.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Hey @Sagar and everyone,
>> >>>>>>>>>>>> Please have a look at the new version and share your
>> thoughts.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>
>> >>>>
>> >>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> On Tue, Jul 18, 2023 at 9:53 AM Sagar <
>> >>> sagarmeansocean@gmail.com>
>> >>>>>>>>>> wrote:
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>> Hi Jack,
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> Thanks for the KIP! Seems like an interesting idea. I have
>> >>> some
>> >>>>>>>>>>> feedback:
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> 1) It would be great if you could clean up the text that
>> >>> seems to
>> >>>>>>>>>> mimic
>> >>>>>>>>>>>>> the
>> >>>>>>>>>>>>> KIP template. It is generally not required in the KIP.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> 2) In the Public Interfaces where you mentioned *Partitioner
>> >>>>>>>> method
>> >>>>>>>>> in
>> >>>>>>>>>>>>> **org/apache/kafka/clients/producer
>> >>>>>>>>>>>>> will have the following update*, I believe you meant the
>> >>>>>>>> Partitioner
>> >>>>>>>>>>>>> *interface*?
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> 3) Staying on Public Interface, it is generally preferable
>> to
>> >>> add
>> >>>>>>>> a
>> >>>>>>>>>>>>> Javadocs section along with the newly added method. You
>> could
>> >>>> also
>> >>>>>>>>>>>>> describe
>> >>>>>>>>>>>>> the behaviour of it invoking the default existing method.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> 4) The option that is mentioned in the Rejected
>> Alternatives,
>> >>>>>>>> seems
>> >>>>>>>>>> more
>> >>>>>>>>>>>>> like a workaround to the current problem that you are
>> >>> describing.
>> >>>>>>>>> That
>> >>>>>>>>>>>>> could be added to the Motivation section IMO.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> 5) Can you also add some more examples of scenarios where
>> this
>> >>>>>>>> would
>> >>>>>>>>>> be
>> >>>>>>>>>>>>> helpful? The only scenario mentioned seems to have a
>> >>> workaround.
>> >>>>>>>>> Just
>> >>>>>>>>>>>>> trying to ensure that we have a strong enough motivation
>> >>> before
>> >>>>>>>>>> adding a
>> >>>>>>>>>>>>> public API.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> 6) One thing which should also be worth noting down would be
>> >>> what
>> >>>>>>>>>>> happens
>> >>>>>>>>>>>>> if users override both methods, only one method (new or old)
>> >>> and
>> >>>>>>>> no
>> >>>>>>>>>>>>> methods
>> >>>>>>>>>>>>> (the default behaviour). It would help in understanding the
>> >>>>>>>> proposal
>> >>>>>>>>>>>>> better.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> Thanks!
>> >>>>>>>>>>>>> Sagar.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> On Mon, Jul 17, 2023 at 9:19 PM Jack Tomy <
>> >>> jacktomy107@gmail.com
>> >>>>>
>> >>>>>>>>>>> wrote:
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>>> Hey everyone,
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> Not seeing much discussion on the KPI. Might be because it
>> is
>> >>>>>>>> too
>> >>>>>>>>>>>>>> obvious 😉.
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> If there are no more comments, I will start the VOTE in the
>> >>>>>>>> coming
>> >>>>>>>>>>> days.
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> On Sat, Jul 15, 2023 at 8:48 PM Jack Tomy <
>> >>>>>>>> jacktomy107@gmail.com>
>> >>>>>>>>>>>>> wrote:
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> Hey everyone,
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> Please take a look at the KPI below and provide your
>> >>>>>>>> suggestions
>> >>>>>>>>>> and
>> >>>>>>>>>>>>>>> feedback. TIA.
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>
>> >>>>
>> >>>
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>> --
>> >>>>>>>>>>>>>>> Best Regards
>> >>>>>>>>>>>>>>> *Jack*
>> >>>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>> --
>> >>>>>>>>>>>>>> Best Regards
>> >>>>>>>>>>>>>> *Jack*
>> >>>>>>>>>>>>>>
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> --
>> >>>>>>>>>>>> Best Regards
>> >>>>>>>>>>>> *Jack*
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>> --
>> >>>>>>>>>>> Best Regards
>> >>>>>>>>>>> *Jack*
>> >>>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>> --
>> >>>>>>>>> Best Regards
>> >>>>>>>>> *Jack*
>> >>>>>>>>>
>> >>>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> --
>> >>>>>>> Best Regards
>> >>>>>>> *Jack*
>> >>>>>>
>> >>>>>>
>> >>>>
>> >>>>
>> >>>
>> >>
>> >>
>> >> --
>> >> Best Regards
>> >> *Jack*
>> >>
>> >
>> >
>> > --
>> > Best Regards
>> > *Jack*
>>
>>
>>
>
> --
> Best Regards
> *Jack*
>
--
Best Regards
*Jack*
Re: [DISCUSS] KIP-953: partition method to be overloaded to accept headers as well.
Posted by Jack Tomy <ja...@gmail.com>.
All right, Thanks Andrew.
Hey everyone,
Please share your thoughts and feedback on the KIP :
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
On Fri, Aug 4, 2023 at 2:50 AM Andrew Schofield <
andrew_schofield_jira@outlook.com> wrote:
> Hi Jack,
> I do understand the idea of extending the Partitioner interface so that
> people are now able to use headers in the partitioning decision, and I see
> that it’s filling in gap in the interface which was left when headers were
> originally added.
>
> Experience with non-default partitioning schemes in the past makes me
> unlikely to use anything other than the default partitioning scheme.
> But I wouldn’t let that dissuade you.
>
> Thanks,
> Andrew
>
> > On 3 Aug 2023, at 13:43, Jack Tomy <ja...@gmail.com> wrote:
> >
> > Hey Andrew, Sagar
> >
> > Please share your thoughts. Thanks.
> >
> >
> >
> > On Mon, Jul 31, 2023 at 5:58 PM Jack Tomy <ja...@gmail.com> wrote:
> >
> >> Hey Andrew, Sagar
> >>
> >> Thanks. I'm travelling so sorry for being brief and getting back late.
> >>
> >> 1. For the first concern, that is moving in a direction of server side
> >> partitioner, the idea seems very much promising but I believe we still
> have
> >> a long way to go. Since the proposal/design for the same is still not
> >> available, it's hard for me to defend my proposal.
> >> 2. For the second concern:
> >> 2.1 Loss of order in messages, I believe the ordering of messages is
> >> never promised and the partitioner has no requirement to ensure the
> same.
> >> It is upto the user to implement/use a partitioner which ensures
> ordering
> >> based on keys.
> >> 2.2 Key deciding the partitioner, It is totally up to the user to decide
> >> the partition regardless of the key, we are also passing the value to
> the
> >> partitioner. Even the existing implementation receives the value which
> lets
> >> the user decide the partition based on value.
> >> 2.3 Sending to a specific partition, for this, I need to be aware of the
> >> total number of partitions, but if I can do that same in partitioner,
> the
> >> cluster param gives me all the information I want.
> >>
> >> I would also quote a line from KIP-82 where headers were added to the
> >> serializer : The payload is traditionally for the business object, and
> *headers
> >> are traditionally used for transport routing*, filtering etc. So I
> >> believe when a user wants to add some routing information (in this case
> >> which set of partitions to go for), headers seem to be the right place.
> >>
> >>
> >>
> >> On Sat, Jul 29, 2023 at 8:48 PM Sagar <sa...@gmail.com>
> wrote:
> >>
> >>> Hi Andrew,
> >>>
> >>> Thanks for your comments.
> >>>
> >>> 1) Yes that makes sense and that's what even would expect to see as
> well.
> >>> I
> >>> just wanted to highlight that we might still need a way to let client
> side
> >>> partitioning logic be present as well. Anyways, I am good on this
> point.
> >>> 2) The example provided does seem achievable by simply attaching the
> >>> partition number in the ProducerRecord. I guess if we can't find any
> >>> further examples which strengthen the case of this partitioner, it
> might
> >>> be
> >>> harder to justify adding it.
> >>>
> >>>
> >>> Thanks!
> >>> Sagar.
> >>>
> >>> On Fri, Jul 28, 2023 at 2:05 PM Andrew Schofield <
> >>> andrew_schofield_jira@outlook.com> wrote:
> >>>
> >>>> Hi Sagar,
> >>>> Thanks for your comments.
> >>>>
> >>>> 1) Server-side partitioning doesn’t necessarily mean that there’s only
> >>> one
> >>>> way to do it. It just means that the partitioning logic runs on the
> >>> broker
> >>>> and
> >>>> any configuration of partitioning applies to the broker’s partitioner.
> >>> If
> >>>> we ever
> >>>> see a KIP for this, that’s the kind of thing I would expect to see.
> >>>>
> >>>> 2) In the priority example in the KIP, there is a kind of contract
> >>> between
> >>>> the
> >>>> producers and consumers so that some records can be processed before
> >>>> others regardless of the order in which they were sent. The producer
> >>>> wants to apply special significance to a particular header to control
> >>> which
> >>>> partition is used. I would simply achieve this by setting the
> partition
> >>>> number
> >>>> in the ProducerRecord at the time of sending.
> >>>>
> >>>> I don’t think the KIP proposes adjusting the built-in partitioner or
> >>>> adding to AK
> >>>> a new one that uses headers in the partitioning decision. So, any
> >>>> configuration
> >>>> for a partitioner that does support headers would be up to the
> >>>> implementation
> >>>> of that specific partitioner. Partitioner implements Configurable.
> >>>>
> >>>> I’m just providing an alternative view and I’m not particularly
> opposed
> >>> to
> >>>> the KIP.
> >>>> I just don’t think it quite merits the work involved to get it voted
> and
> >>>> merged.
> >>>> As an aside, a long time ago, I created a small KIP that was never
> >>> adopted
> >>>> and I didn’t push it because I eventually didn’t need it.
> >>>>
> >>>> Thanks,
> >>>> Andrew
> >>>>
> >>>>> On 28 Jul 2023, at 05:15, Sagar <sa...@gmail.com> wrote:
> >>>>>
> >>>>> Hey Andrew,
> >>>>>
> >>>>> Thanks for the review. Since I had reviewed the KIP I thought I would
> >>>> also
> >>>>> respond. Of course Jack has the final say on this since he wrote the
> >>> KIP.
> >>>>>
> >>>>> 1) This is an interesting point and I hadn't considered it. The
> >>>>> comparison with KIP-848 is a valid one but even within that KIP, it
> >>>> allows
> >>>>> client side partitioning for power users like Streams. So while we
> >>> would
> >>>>> want to move away from client side partitioner as much as possible,
> we
> >>>>> still shouldn't do away completely with Client side partitioning and
> >>> end
> >>>> up
> >>>>> being in a state of inflexibility for different kinds of usecases.
> >>> This
> >>>> is
> >>>>> my opinion though and you have more context on Clients, so would like
> >>> to
> >>>>> know your thoughts on this.
> >>>>>
> >>>>> 2) Regarding this, I assumed that since the headers are already part
> >>> of
> >>>> the
> >>>>> consumer records they should have access to the headers and if there
> >>> is a
> >>>>> contract b/w the applications producing and the application
> consuming,
> >>>> that
> >>>>> decisioning should be transparent. Was my assumption incorrect? But
> as
> >>>> you
> >>>>> rightly pointed out header based partitioning with keys is going to
> >>> lead
> >>>> to
> >>>>> surprising results. Assuming there is merit in this proposal, do you
> >>>> think
> >>>>> we should ignore the keys in this case (similar to the effect of
> >>>>> setting *partitioner.ignore.keys
> >>>>> *config to false) and document it appropriately?
> >>>>>
> >>>>> Let me know what you think.
> >>>>>
> >>>>> Thanks!
> >>>>> Sagar.
> >>>>>
> >>>>>
> >>>>> On Thu, Jul 27, 2023 at 9:41 PM Andrew Schofield <
> >>>>> andrew_schofield_jira@outlook.com> wrote:
> >>>>>
> >>>>>> Hi Jack,
> >>>>>> Thanks for the KIP. I have a few concerns about the idea.
> >>>>>>
> >>>>>> 1) I think that while a client-side partitioner seems like a neat
> >>> idea
> >>>> and
> >>>>>> it’s an established part of Kafka,
> >>>>>> it’s one of the things which makes Kafka clients quite complicated.
> >>> Just
> >>>>>> as KIP-848 is moving from
> >>>>>> client-side assignors to server-side assignors, I wonder whether
> >>> really
> >>>> we
> >>>>>> should be looking to make
> >>>>>> partitioning a server-side capability too over time. So, I’m not
> >>>> convinced
> >>>>>> that making the Partitioner
> >>>>>> interface richer is moving in the right direction.
> >>>>>>
> >>>>>> 2) For records with a key, the partitioner usually calculates the
> >>>>>> partition from the key. This means
> >>>>>> that records with the same key end up on the same partition. Many
> >>>>>> applications expect this to give ordering.
> >>>>>> Log compaction expects this. There are situations in which records
> >>> have
> >>>> to
> >>>>>> be repartitioned, such as
> >>>>>> sometimes happens with Kafka Streams. I think that a header-based
> >>>>>> partitioner for records which have
> >>>>>> keys is going to be surprising and only going to have limited
> >>>>>> applicability as a result.
> >>>>>>
> >>>>>> The tricky part about clever partitioning is that downstream systems
> >>>> have
> >>>>>> no idea how the partition
> >>>>>> number was arrived at, so they do not truly understand how the
> >>> ordering
> >>>>>> was derived. I do think that
> >>>>>> perhaps there’s value to being able to influence the partitioning
> >>> using
> >>>>>> the headers, but I wonder if actually
> >>>>>> transforming the headers into an “ordering context” that then flows
> >>> with
> >>>>>> the record as it moves through
> >>>>>> the system would be a stronger solution. Today, the key is the
> >>> ordering
> >>>>>> context. Maybe it should be a
> >>>>>> concept in its own right and the Producer could configure a
> converter
> >>>> from
> >>>>>> headers to ordering context.
> >>>>>> That is of course a much bigger change.
> >>>>>>
> >>>>>> In one of the examples you mention in the KIP, you mentioned using a
> >>>>>> header to control priority. I guess the
> >>>>>> idea is to preferentially process records off specific partitions so
> >>>> they
> >>>>>> can overtake lower priority records.
> >>>>>> I suggest just sending the records explicitly to specific partitions
> >>> to
> >>>>>> achieve this.
> >>>>>>
> >>>>>> Sorry for the essay, but you did ask for people to share their
> >>> thoughts
> >>>> :)
> >>>>>>
> >>>>>> Just my opinion. Let’s see what others think.
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Andrew
> >>>>>>
> >>>>>>> On 25 Jul 2023, at 14:58, Jack Tomy <ja...@gmail.com> wrote:
> >>>>>>>
> >>>>>>> Hey @Sagar
> >>>>>>>
> >>>>>>> Thanks again for the review.
> >>>>>>> 1. "a null headers value is equivalent to invoking the older
> >>> partition
> >>>>>>> method", this is not true. If someone makes an implementation and
> >>> the
> >>>>>>> headers come as null, still the new implementation will take
> effect.
> >>>>>>> Instead I have added : "Not overriding this method in the
> >>> Partitioner
> >>>>>>> interface has the same behaviour as using the existing method."
> >>>>>>> 2. Corrected.
> >>>>>>>
> >>>>>>> Hey @Sagar and everyone,
> >>>>>>> Please have a look at the new version and share your thoughts.
> >>>>>>>
> >>>>>>
> >>>>
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
> >>>>>>> Like Sagar mentioned, I would also request more people who have
> more
> >>>>>>> context on clients to chime in.
> >>>>>>>
> >>>>>>>
> >>>>>>> On Tue, Jul 25, 2023 at 2:58 PM Sagar <sa...@gmail.com>
> >>>> wrote:
> >>>>>>>
> >>>>>>>> Hi Jack,
> >>>>>>>>
> >>>>>>>> Thanks I have a couple of final comments and then I am good.
> >>>>>>>>
> >>>>>>>> 1) Can you elaborate on the Javadocs of the partition headers
> >>> argument
> >>>>>> to
> >>>>>>>> specify that a null headers value is equivalent to invoking the
> >>> older
> >>>>>>>> partition method? It is apparent but generally good to call out.
> >>>>>>>> 2) In the Compatibility section, you have mentioned backward
> >>>>>> comparable. I
> >>>>>>>> believe it should be *backward compatible change.*
> >>>>>>>>
> >>>>>>>> I don't have other comments. Post this, probably someone else who
> >>> has
> >>>>>> more
> >>>>>>>> context on Clients can also chime in on this before we can move
> >>> this
> >>>> to
> >>>>>>>> Voting.
> >>>>>>>>
> >>>>>>>> Thanks!
> >>>>>>>> Sagar.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Sat, Jul 22, 2023 at 10:09 AM Jack Tomy <jacktomy107@gmail.com
> >
> >>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Hey @Sagar,
> >>>>>>>>>
> >>>>>>>>> Thank you again for the response and feedback.
> >>>>>>>>>
> >>>>>>>>> 1. Though the ask wasn't very clear to me I have attached the
> >>>> Javadoc
> >>>>>>>> as
> >>>>>>>>> per your suggestion. Please have a look and let me know if this
> >>>> meets
> >>>>>>>>> the
> >>>>>>>>> expectations.
> >>>>>>>>> 2. Done.
> >>>>>>>>> 3. Done
> >>>>>>>>> 4. Done
> >>>>>>>>>
> >>>>>>>>> Hey @Sagar and everyone,
> >>>>>>>>> Please have a look at the new version and share your thoughts.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
> >>>>>>>>>
> >>>>>>>>> On Thu, Jul 20, 2023 at 9:46 PM Sagar <sagarmeansocean@gmail.com
> >
> >>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>> Thanks Jack for the updates.
> >>>>>>>>>>
> >>>>>>>>>> Some more feedback:
> >>>>>>>>>>
> >>>>>>>>>> 1) It would be better if you can add the Javadoc in the Public
> >>>>>>>> interfaces
> >>>>>>>>>> section. That is a general practice used which gives the readers
> >>> of
> >>>>>> the
> >>>>>>>>> KIP
> >>>>>>>>>> a high level idea of the Public Interfaces.
> >>>>>>>>>>
> >>>>>>>>>> 2) In the proposed section, the bit about marking headers as
> read
> >>>> only
> >>>>>>>>>> seems like an implementation detail This can generally be
> >>> avoided in
> >>>>>>>>> KIPs.
> >>>>>>>>>>
> >>>>>>>>>> 3) Also, in the Deprecation section, can you mention again that
> >>> this
> >>>>>>>> is a
> >>>>>>>>>> backward compatible change and the reason for it (already done
> in
> >>>> the
> >>>>>>>>>> Proposed Changes section).
> >>>>>>>>>>
> >>>>>>>>>> 4) In the Testing Plan section, there is still the KIP template
> >>> bit
> >>>>>>>>> copied
> >>>>>>>>>> over. That can be removed.
> >>>>>>>>>>
> >>>>>>>>>> Thanks!
> >>>>>>>>>> Sagar.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> On Thu, Jul 20, 2023 at 2:48 PM Jack Tomy <
> jacktomy107@gmail.com
> >>>>
> >>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> Hey Everyone,
> >>>>>>>>>>>
> >>>>>>>>>>> Please consider this as a reminder and share your feedback.
> >>> Thank
> >>>>>>>> you.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
> >>>>>>>>>>>
> >>>>>>>>>>> On Tue, Jul 18, 2023 at 5:43 PM Jack Tomy <
> >>> jacktomy107@gmail.com>
> >>>>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>> Hey @Sagar,
> >>>>>>>>>>>>
> >>>>>>>>>>>> Thank you for the response and feedback.
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. Done
> >>>>>>>>>>>> 2. Yeah, that was a mistake from my end. Corrected.
> >>>>>>>>>>>> 3. Can you please elaborate this, I have added the java doc
> >>>>>>>> along
> >>>>>>>>>> with
> >>>>>>>>>>>> the code changes. Should I paste the same in KIP too?
> >>>>>>>>>>>> 4. Moved.
> >>>>>>>>>>>> 5. I have added one more use case, it is actually helpful in
> >>> any
> >>>>>>>>>>>> situation where you want to pass some information to partition
> >>>>>>>>>> method
> >>>>>>>>>>> but
> >>>>>>>>>>>> don't have to have it in the key or value.
> >>>>>>>>>>>> 6. Added.
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Hey @Sagar and everyone,
> >>>>>>>>>>>> Please have a look at the new version and share your thoughts.
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Tue, Jul 18, 2023 at 9:53 AM Sagar <
> >>> sagarmeansocean@gmail.com>
> >>>>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Hi Jack,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Thanks for the KIP! Seems like an interesting idea. I have
> >>> some
> >>>>>>>>>>> feedback:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 1) It would be great if you could clean up the text that
> >>> seems to
> >>>>>>>>>> mimic
> >>>>>>>>>>>>> the
> >>>>>>>>>>>>> KIP template. It is generally not required in the KIP.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 2) In the Public Interfaces where you mentioned *Partitioner
> >>>>>>>> method
> >>>>>>>>> in
> >>>>>>>>>>>>> **org/apache/kafka/clients/producer
> >>>>>>>>>>>>> will have the following update*, I believe you meant the
> >>>>>>>> Partitioner
> >>>>>>>>>>>>> *interface*?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 3) Staying on Public Interface, it is generally preferable to
> >>> add
> >>>>>>>> a
> >>>>>>>>>>>>> Javadocs section along with the newly added method. You could
> >>>> also
> >>>>>>>>>>>>> describe
> >>>>>>>>>>>>> the behaviour of it invoking the default existing method.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 4) The option that is mentioned in the Rejected Alternatives,
> >>>>>>>> seems
> >>>>>>>>>> more
> >>>>>>>>>>>>> like a workaround to the current problem that you are
> >>> describing.
> >>>>>>>>> That
> >>>>>>>>>>>>> could be added to the Motivation section IMO.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 5) Can you also add some more examples of scenarios where
> this
> >>>>>>>> would
> >>>>>>>>>> be
> >>>>>>>>>>>>> helpful? The only scenario mentioned seems to have a
> >>> workaround.
> >>>>>>>>> Just
> >>>>>>>>>>>>> trying to ensure that we have a strong enough motivation
> >>> before
> >>>>>>>>>> adding a
> >>>>>>>>>>>>> public API.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 6) One thing which should also be worth noting down would be
> >>> what
> >>>>>>>>>>> happens
> >>>>>>>>>>>>> if users override both methods, only one method (new or old)
> >>> and
> >>>>>>>> no
> >>>>>>>>>>>>> methods
> >>>>>>>>>>>>> (the default behaviour). It would help in understanding the
> >>>>>>>> proposal
> >>>>>>>>>>>>> better.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Thanks!
> >>>>>>>>>>>>> Sagar.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Mon, Jul 17, 2023 at 9:19 PM Jack Tomy <
> >>> jacktomy107@gmail.com
> >>>>>
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> Hey everyone,
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Not seeing much discussion on the KPI. Might be because it
> is
> >>>>>>>> too
> >>>>>>>>>>>>>> obvious 😉.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> If there are no more comments, I will start the VOTE in the
> >>>>>>>> coming
> >>>>>>>>>>> days.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Sat, Jul 15, 2023 at 8:48 PM Jack Tomy <
> >>>>>>>> jacktomy107@gmail.com>
> >>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Hey everyone,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Please take a look at the KPI below and provide your
> >>>>>>>> suggestions
> >>>>>>>>>> and
> >>>>>>>>>>>>>>> feedback. TIA.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>
> >>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> --
> >>>>>>>>>>>>>>> Best Regards
> >>>>>>>>>>>>>>> *Jack*
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> --
> >>>>>>>>>>>>>> Best Regards
> >>>>>>>>>>>>>> *Jack*
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> --
> >>>>>>>>>>>> Best Regards
> >>>>>>>>>>>> *Jack*
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> --
> >>>>>>>>>>> Best Regards
> >>>>>>>>>>> *Jack*
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> --
> >>>>>>>>> Best Regards
> >>>>>>>>> *Jack*
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Best Regards
> >>>>>>> *Jack*
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>>
> >>
> >>
> >> --
> >> Best Regards
> >> *Jack*
> >>
> >
> >
> > --
> > Best Regards
> > *Jack*
>
>
>
--
Best Regards
*Jack*
Re: [DISCUSS] KIP-953: partition method to be overloaded to accept headers as well.
Posted by Andrew Schofield <an...@outlook.com>.
Hi Jack,
I do understand the idea of extending the Partitioner interface so that
people are now able to use headers in the partitioning decision, and I see
that it’s filling in gap in the interface which was left when headers were
originally added.
Experience with non-default partitioning schemes in the past makes me
unlikely to use anything other than the default partitioning scheme.
But I wouldn’t let that dissuade you.
Thanks,
Andrew
> On 3 Aug 2023, at 13:43, Jack Tomy <ja...@gmail.com> wrote:
>
> Hey Andrew, Sagar
>
> Please share your thoughts. Thanks.
>
>
>
> On Mon, Jul 31, 2023 at 5:58 PM Jack Tomy <ja...@gmail.com> wrote:
>
>> Hey Andrew, Sagar
>>
>> Thanks. I'm travelling so sorry for being brief and getting back late.
>>
>> 1. For the first concern, that is moving in a direction of server side
>> partitioner, the idea seems very much promising but I believe we still have
>> a long way to go. Since the proposal/design for the same is still not
>> available, it's hard for me to defend my proposal.
>> 2. For the second concern:
>> 2.1 Loss of order in messages, I believe the ordering of messages is
>> never promised and the partitioner has no requirement to ensure the same.
>> It is upto the user to implement/use a partitioner which ensures ordering
>> based on keys.
>> 2.2 Key deciding the partitioner, It is totally up to the user to decide
>> the partition regardless of the key, we are also passing the value to the
>> partitioner. Even the existing implementation receives the value which lets
>> the user decide the partition based on value.
>> 2.3 Sending to a specific partition, for this, I need to be aware of the
>> total number of partitions, but if I can do that same in partitioner, the
>> cluster param gives me all the information I want.
>>
>> I would also quote a line from KIP-82 where headers were added to the
>> serializer : The payload is traditionally for the business object, and *headers
>> are traditionally used for transport routing*, filtering etc. So I
>> believe when a user wants to add some routing information (in this case
>> which set of partitions to go for), headers seem to be the right place.
>>
>>
>>
>> On Sat, Jul 29, 2023 at 8:48 PM Sagar <sa...@gmail.com> wrote:
>>
>>> Hi Andrew,
>>>
>>> Thanks for your comments.
>>>
>>> 1) Yes that makes sense and that's what even would expect to see as well.
>>> I
>>> just wanted to highlight that we might still need a way to let client side
>>> partitioning logic be present as well. Anyways, I am good on this point.
>>> 2) The example provided does seem achievable by simply attaching the
>>> partition number in the ProducerRecord. I guess if we can't find any
>>> further examples which strengthen the case of this partitioner, it might
>>> be
>>> harder to justify adding it.
>>>
>>>
>>> Thanks!
>>> Sagar.
>>>
>>> On Fri, Jul 28, 2023 at 2:05 PM Andrew Schofield <
>>> andrew_schofield_jira@outlook.com> wrote:
>>>
>>>> Hi Sagar,
>>>> Thanks for your comments.
>>>>
>>>> 1) Server-side partitioning doesn’t necessarily mean that there’s only
>>> one
>>>> way to do it. It just means that the partitioning logic runs on the
>>> broker
>>>> and
>>>> any configuration of partitioning applies to the broker’s partitioner.
>>> If
>>>> we ever
>>>> see a KIP for this, that’s the kind of thing I would expect to see.
>>>>
>>>> 2) In the priority example in the KIP, there is a kind of contract
>>> between
>>>> the
>>>> producers and consumers so that some records can be processed before
>>>> others regardless of the order in which they were sent. The producer
>>>> wants to apply special significance to a particular header to control
>>> which
>>>> partition is used. I would simply achieve this by setting the partition
>>>> number
>>>> in the ProducerRecord at the time of sending.
>>>>
>>>> I don’t think the KIP proposes adjusting the built-in partitioner or
>>>> adding to AK
>>>> a new one that uses headers in the partitioning decision. So, any
>>>> configuration
>>>> for a partitioner that does support headers would be up to the
>>>> implementation
>>>> of that specific partitioner. Partitioner implements Configurable.
>>>>
>>>> I’m just providing an alternative view and I’m not particularly opposed
>>> to
>>>> the KIP.
>>>> I just don’t think it quite merits the work involved to get it voted and
>>>> merged.
>>>> As an aside, a long time ago, I created a small KIP that was never
>>> adopted
>>>> and I didn’t push it because I eventually didn’t need it.
>>>>
>>>> Thanks,
>>>> Andrew
>>>>
>>>>> On 28 Jul 2023, at 05:15, Sagar <sa...@gmail.com> wrote:
>>>>>
>>>>> Hey Andrew,
>>>>>
>>>>> Thanks for the review. Since I had reviewed the KIP I thought I would
>>>> also
>>>>> respond. Of course Jack has the final say on this since he wrote the
>>> KIP.
>>>>>
>>>>> 1) This is an interesting point and I hadn't considered it. The
>>>>> comparison with KIP-848 is a valid one but even within that KIP, it
>>>> allows
>>>>> client side partitioning for power users like Streams. So while we
>>> would
>>>>> want to move away from client side partitioner as much as possible, we
>>>>> still shouldn't do away completely with Client side partitioning and
>>> end
>>>> up
>>>>> being in a state of inflexibility for different kinds of usecases.
>>> This
>>>> is
>>>>> my opinion though and you have more context on Clients, so would like
>>> to
>>>>> know your thoughts on this.
>>>>>
>>>>> 2) Regarding this, I assumed that since the headers are already part
>>> of
>>>> the
>>>>> consumer records they should have access to the headers and if there
>>> is a
>>>>> contract b/w the applications producing and the application consuming,
>>>> that
>>>>> decisioning should be transparent. Was my assumption incorrect? But as
>>>> you
>>>>> rightly pointed out header based partitioning with keys is going to
>>> lead
>>>> to
>>>>> surprising results. Assuming there is merit in this proposal, do you
>>>> think
>>>>> we should ignore the keys in this case (similar to the effect of
>>>>> setting *partitioner.ignore.keys
>>>>> *config to false) and document it appropriately?
>>>>>
>>>>> Let me know what you think.
>>>>>
>>>>> Thanks!
>>>>> Sagar.
>>>>>
>>>>>
>>>>> On Thu, Jul 27, 2023 at 9:41 PM Andrew Schofield <
>>>>> andrew_schofield_jira@outlook.com> wrote:
>>>>>
>>>>>> Hi Jack,
>>>>>> Thanks for the KIP. I have a few concerns about the idea.
>>>>>>
>>>>>> 1) I think that while a client-side partitioner seems like a neat
>>> idea
>>>> and
>>>>>> it’s an established part of Kafka,
>>>>>> it’s one of the things which makes Kafka clients quite complicated.
>>> Just
>>>>>> as KIP-848 is moving from
>>>>>> client-side assignors to server-side assignors, I wonder whether
>>> really
>>>> we
>>>>>> should be looking to make
>>>>>> partitioning a server-side capability too over time. So, I’m not
>>>> convinced
>>>>>> that making the Partitioner
>>>>>> interface richer is moving in the right direction.
>>>>>>
>>>>>> 2) For records with a key, the partitioner usually calculates the
>>>>>> partition from the key. This means
>>>>>> that records with the same key end up on the same partition. Many
>>>>>> applications expect this to give ordering.
>>>>>> Log compaction expects this. There are situations in which records
>>> have
>>>> to
>>>>>> be repartitioned, such as
>>>>>> sometimes happens with Kafka Streams. I think that a header-based
>>>>>> partitioner for records which have
>>>>>> keys is going to be surprising and only going to have limited
>>>>>> applicability as a result.
>>>>>>
>>>>>> The tricky part about clever partitioning is that downstream systems
>>>> have
>>>>>> no idea how the partition
>>>>>> number was arrived at, so they do not truly understand how the
>>> ordering
>>>>>> was derived. I do think that
>>>>>> perhaps there’s value to being able to influence the partitioning
>>> using
>>>>>> the headers, but I wonder if actually
>>>>>> transforming the headers into an “ordering context” that then flows
>>> with
>>>>>> the record as it moves through
>>>>>> the system would be a stronger solution. Today, the key is the
>>> ordering
>>>>>> context. Maybe it should be a
>>>>>> concept in its own right and the Producer could configure a converter
>>>> from
>>>>>> headers to ordering context.
>>>>>> That is of course a much bigger change.
>>>>>>
>>>>>> In one of the examples you mention in the KIP, you mentioned using a
>>>>>> header to control priority. I guess the
>>>>>> idea is to preferentially process records off specific partitions so
>>>> they
>>>>>> can overtake lower priority records.
>>>>>> I suggest just sending the records explicitly to specific partitions
>>> to
>>>>>> achieve this.
>>>>>>
>>>>>> Sorry for the essay, but you did ask for people to share their
>>> thoughts
>>>> :)
>>>>>>
>>>>>> Just my opinion. Let’s see what others think.
>>>>>>
>>>>>> Thanks,
>>>>>> Andrew
>>>>>>
>>>>>>> On 25 Jul 2023, at 14:58, Jack Tomy <ja...@gmail.com> wrote:
>>>>>>>
>>>>>>> Hey @Sagar
>>>>>>>
>>>>>>> Thanks again for the review.
>>>>>>> 1. "a null headers value is equivalent to invoking the older
>>> partition
>>>>>>> method", this is not true. If someone makes an implementation and
>>> the
>>>>>>> headers come as null, still the new implementation will take effect.
>>>>>>> Instead I have added : "Not overriding this method in the
>>> Partitioner
>>>>>>> interface has the same behaviour as using the existing method."
>>>>>>> 2. Corrected.
>>>>>>>
>>>>>>> Hey @Sagar and everyone,
>>>>>>> Please have a look at the new version and share your thoughts.
>>>>>>>
>>>>>>
>>>>
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>>>>>>> Like Sagar mentioned, I would also request more people who have more
>>>>>>> context on clients to chime in.
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jul 25, 2023 at 2:58 PM Sagar <sa...@gmail.com>
>>>> wrote:
>>>>>>>
>>>>>>>> Hi Jack,
>>>>>>>>
>>>>>>>> Thanks I have a couple of final comments and then I am good.
>>>>>>>>
>>>>>>>> 1) Can you elaborate on the Javadocs of the partition headers
>>> argument
>>>>>> to
>>>>>>>> specify that a null headers value is equivalent to invoking the
>>> older
>>>>>>>> partition method? It is apparent but generally good to call out.
>>>>>>>> 2) In the Compatibility section, you have mentioned backward
>>>>>> comparable. I
>>>>>>>> believe it should be *backward compatible change.*
>>>>>>>>
>>>>>>>> I don't have other comments. Post this, probably someone else who
>>> has
>>>>>> more
>>>>>>>> context on Clients can also chime in on this before we can move
>>> this
>>>> to
>>>>>>>> Voting.
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>> Sagar.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Sat, Jul 22, 2023 at 10:09 AM Jack Tomy <ja...@gmail.com>
>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hey @Sagar,
>>>>>>>>>
>>>>>>>>> Thank you again for the response and feedback.
>>>>>>>>>
>>>>>>>>> 1. Though the ask wasn't very clear to me I have attached the
>>>> Javadoc
>>>>>>>> as
>>>>>>>>> per your suggestion. Please have a look and let me know if this
>>>> meets
>>>>>>>>> the
>>>>>>>>> expectations.
>>>>>>>>> 2. Done.
>>>>>>>>> 3. Done
>>>>>>>>> 4. Done
>>>>>>>>>
>>>>>>>>> Hey @Sagar and everyone,
>>>>>>>>> Please have a look at the new version and share your thoughts.
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>>>>>>>>>
>>>>>>>>> On Thu, Jul 20, 2023 at 9:46 PM Sagar <sa...@gmail.com>
>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Thanks Jack for the updates.
>>>>>>>>>>
>>>>>>>>>> Some more feedback:
>>>>>>>>>>
>>>>>>>>>> 1) It would be better if you can add the Javadoc in the Public
>>>>>>>> interfaces
>>>>>>>>>> section. That is a general practice used which gives the readers
>>> of
>>>>>> the
>>>>>>>>> KIP
>>>>>>>>>> a high level idea of the Public Interfaces.
>>>>>>>>>>
>>>>>>>>>> 2) In the proposed section, the bit about marking headers as read
>>>> only
>>>>>>>>>> seems like an implementation detail This can generally be
>>> avoided in
>>>>>>>>> KIPs.
>>>>>>>>>>
>>>>>>>>>> 3) Also, in the Deprecation section, can you mention again that
>>> this
>>>>>>>> is a
>>>>>>>>>> backward compatible change and the reason for it (already done in
>>>> the
>>>>>>>>>> Proposed Changes section).
>>>>>>>>>>
>>>>>>>>>> 4) In the Testing Plan section, there is still the KIP template
>>> bit
>>>>>>>>> copied
>>>>>>>>>> over. That can be removed.
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>> Sagar.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Thu, Jul 20, 2023 at 2:48 PM Jack Tomy <jacktomy107@gmail.com
>>>>
>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hey Everyone,
>>>>>>>>>>>
>>>>>>>>>>> Please consider this as a reminder and share your feedback.
>>> Thank
>>>>>>>> you.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Jul 18, 2023 at 5:43 PM Jack Tomy <
>>> jacktomy107@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hey @Sagar,
>>>>>>>>>>>>
>>>>>>>>>>>> Thank you for the response and feedback.
>>>>>>>>>>>>
>>>>>>>>>>>> 1. Done
>>>>>>>>>>>> 2. Yeah, that was a mistake from my end. Corrected.
>>>>>>>>>>>> 3. Can you please elaborate this, I have added the java doc
>>>>>>>> along
>>>>>>>>>> with
>>>>>>>>>>>> the code changes. Should I paste the same in KIP too?
>>>>>>>>>>>> 4. Moved.
>>>>>>>>>>>> 5. I have added one more use case, it is actually helpful in
>>> any
>>>>>>>>>>>> situation where you want to pass some information to partition
>>>>>>>>>> method
>>>>>>>>>>> but
>>>>>>>>>>>> don't have to have it in the key or value.
>>>>>>>>>>>> 6. Added.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hey @Sagar and everyone,
>>>>>>>>>>>> Please have a look at the new version and share your thoughts.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Jul 18, 2023 at 9:53 AM Sagar <
>>> sagarmeansocean@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Jack,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for the KIP! Seems like an interesting idea. I have
>>> some
>>>>>>>>>>> feedback:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 1) It would be great if you could clean up the text that
>>> seems to
>>>>>>>>>> mimic
>>>>>>>>>>>>> the
>>>>>>>>>>>>> KIP template. It is generally not required in the KIP.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2) In the Public Interfaces where you mentioned *Partitioner
>>>>>>>> method
>>>>>>>>> in
>>>>>>>>>>>>> **org/apache/kafka/clients/producer
>>>>>>>>>>>>> will have the following update*, I believe you meant the
>>>>>>>> Partitioner
>>>>>>>>>>>>> *interface*?
>>>>>>>>>>>>>
>>>>>>>>>>>>> 3) Staying on Public Interface, it is generally preferable to
>>> add
>>>>>>>> a
>>>>>>>>>>>>> Javadocs section along with the newly added method. You could
>>>> also
>>>>>>>>>>>>> describe
>>>>>>>>>>>>> the behaviour of it invoking the default existing method.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 4) The option that is mentioned in the Rejected Alternatives,
>>>>>>>> seems
>>>>>>>>>> more
>>>>>>>>>>>>> like a workaround to the current problem that you are
>>> describing.
>>>>>>>>> That
>>>>>>>>>>>>> could be added to the Motivation section IMO.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 5) Can you also add some more examples of scenarios where this
>>>>>>>> would
>>>>>>>>>> be
>>>>>>>>>>>>> helpful? The only scenario mentioned seems to have a
>>> workaround.
>>>>>>>>> Just
>>>>>>>>>>>>> trying to ensure that we have a strong enough motivation
>>> before
>>>>>>>>>> adding a
>>>>>>>>>>>>> public API.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 6) One thing which should also be worth noting down would be
>>> what
>>>>>>>>>>> happens
>>>>>>>>>>>>> if users override both methods, only one method (new or old)
>>> and
>>>>>>>> no
>>>>>>>>>>>>> methods
>>>>>>>>>>>>> (the default behaviour). It would help in understanding the
>>>>>>>> proposal
>>>>>>>>>>>>> better.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>> Sagar.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Mon, Jul 17, 2023 at 9:19 PM Jack Tomy <
>>> jacktomy107@gmail.com
>>>>>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hey everyone,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Not seeing much discussion on the KPI. Might be because it is
>>>>>>>> too
>>>>>>>>>>>>>> obvious 😉.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> If there are no more comments, I will start the VOTE in the
>>>>>>>> coming
>>>>>>>>>>> days.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Sat, Jul 15, 2023 at 8:48 PM Jack Tomy <
>>>>>>>> jacktomy107@gmail.com>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hey everyone,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Please take a look at the KPI below and provide your
>>>>>>>> suggestions
>>>>>>>>>> and
>>>>>>>>>>>>>>> feedback. TIA.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263424937
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> Best Regards
>>>>>>>>>>>>>>> *Jack*
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Best Regards
>>>>>>>>>>>>>> *Jack*
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Best Regards
>>>>>>>>>>>> *Jack*
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Best Regards
>>>>>>>>>>> *Jack*
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Best Regards
>>>>>>>>> *Jack*
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Best Regards
>>>>>>> *Jack*
>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>
>>
>> --
>> Best Regards
>> *Jack*
>>
>
>
> --
> Best Regards
> *Jack*