[DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Hi all,

I have submitted KIP-881 to implement rack-aware partition assignment for
consumers:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers.
It adds rack id to the consumer group protocol to propagate rack
information so that rack-aware assignors can be added to benefit from
locality.

Feedback and suggestions are welcome!

Thank you,

Rajini

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Hi all,

With KIP-881, consumer partition assignors attempt to improve locality by
matching consumer racks with partition replica racks. At the moment, we
don't rebalance when replicas change, so changes in replica racks are not
taken into account for partition assignment until the next rebalance, which
could be a very long time. To overcome this limitation, we would like to
also trigger rebalance when metadata indicates that the racks on which a
partition has replicas has changed. The rebalance will be triggered by the
leader if `client.rack` is configured and the set of racks of any of the
partitions changes. Rebalances are not triggered for transient changes like
a replica being in ISR or being temporarily offline. But for less frequent,
but long-lived changes due to reassignment, rebalance will be triggered to
improve locality when rack-awareness is enabled for consumers.

Please let me know if there are any concerns with this change. Otherwise I
will update the KIP and submit a PR.

Thank you,

Rajini


On Thu, Dec 1, 2022 at 9:12 AM Rajini Sivaram <ra...@gmail.com>
wrote:

> Hi Artem,
>
> Yes, clients can reset their `client.rack` config if the broker doesn't
> use a rack-aware selector. This config is only useful for improving
> locality with a rack-aware selector.
>
> On Wed, Nov 30, 2022 at 10:05 PM Artem Livshits
> <al...@confluent.io.invalid> wrote:
>
>> I think it's reasonable for practical scenarios.  Is it possible to turn
>> off rack awareness in the clients in case the broker selector plugin is
>> not
>> compatible with rack-aware logic in the client?
>>
>> -Artem
>>
>> On Wed, Nov 30, 2022 at 2:46 AM Rajini Sivaram <ra...@gmail.com>
>> wrote:
>>
>> > Hi Artem,
>> >
>> > Understood your concern - brokers could use a replica selector that uses
>> > some other client metadata other than rack id to decide the preferred
>> read
>> > replica, or just use the leader. In that case, consumers wouldn't really
>> > benefit from aligning partition assignment on rack ids. So the question
>> is
>> > whether we should make the default consumer assignors use rack ids for
>> > partition assignment or whether we should have different rack-aware
>> > assignors that can be configured when brokers use rack-aware replica
>> > selector. We had a similar discussion earlier in the thread (the KIP had
>> > originally proposed new rack-aware assignors). We decided to update the
>> > default assignors because:
>> > 1) Consumers using fetch-from-follower automatically benefit from
>> improved
>> > locality, without having to update another config.
>> > 2) Consumers configure rack id for improved locality, so aligning on
>> > replica rack ids generally makes sense.
>> > 3) We prioritize balanced assignment over locality in the consumer, so
>> the
>> > default assignors should work effectively regardless of broker's replica
>> > selector.
>> >
>> > Does that make sense?
>> >
>> >
>> > Thank you,
>> >
>> > Rajini
>> >
>> >
>> >
>> > On Tue, Nov 29, 2022 at 1:05 AM Artem Livshits
>> > <al...@confluent.io.invalid> wrote:
>> >
>> > > I'm thinking of a case where the broker uses a plugin that does not
>> use
>> > > rack ids to determine client locality, in that case the consumer might
>> > > arrive at the wrong decision based on rack ids.
>> > >
>> > > -Artem
>> > >
>> > > On Wed, Nov 23, 2022 at 3:54 AM Rajini Sivaram <
>> rajinisivaram@gmail.com>
>> > > wrote:
>> > >
>> > > > Hi Artem,
>> > > >
>> > > > Thanks for reviewing the KIP. The client doesn't rely on a
>> particular
>> > > > replica assignment logic in the broker. Instead, it matches the
>> actual
>> > > rack
>> > > > assignment for partitions from the metadata with consumer racks. So
>> > there
>> > > > is no assumption in the client implementation about the use of
>> > > > RackAwareReplicaSelector in the broker. Did I misunderstand your
>> > concern?
>> > > >
>> > > > Regards,
>> > > >
>> > > > Rajini
>> > > >
>> > > >
>> > > > On Tue, Nov 22, 2022 at 11:03 PM Artem Livshits
>> > > > <al...@confluent.io.invalid> wrote:
>> > > >
>> > > > > Hi Rajini,
>> > > > >
>> > > > > Thank you for the KIP, the KIP looks good to match
>> > > > RackAwareReplicaSelector
>> > > > > behavior that is available out-of-box.  Which should probably be
>> good
>> > > > > enough in practice.
>> > > > >
>> > > > > From the design perspective, though, RackAwareReplicaSelector is
>> just
>> > > one
>> > > > > possible plugin, in theory the broker could use a plugin that
>> > leverages
>> > > > > networking information to get client locality or some other way,
>> so
>> > it
>> > > > > seems like we're making an assumption about broker replica
>> selection
>> > in
>> > > > the
>> > > > > default assignment implementation.  So I wonder if we should use a
>> > > > separate
>> > > > > plugin that would be set when RackAwareReplicaSelector is set,
>> rather
>> > > > than
>> > > > > assume broker behavior in the client implementation.
>> > > > >
>> > > > > -Artem
>> > > > >
>> > > > > On Wed, Nov 16, 2022 at 8:08 AM Jun Rao <jun@confluent.io.invalid
>> >
>> > > > wrote:
>> > > > >
>> > > > > > Hi, David and Rajini,
>> > > > > >
>> > > > > > Thanks for the explanation. It makes sense to me now.
>> > > > > >
>> > > > > > Jun
>> > > > > >
>> > > > > > On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <
>> > > > rajinisivaram@gmail.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > Thanks David, that was my understanding as well.
>> > > > > > >
>> > > > > > > Regards,
>> > > > > > >
>> > > > > > > Rajini
>> > > > > > >
>> > > > > > > On Wed, Nov 16, 2022 at 8:08 AM David Jacot
>> > > > > <djacot@confluent.io.invalid
>> > > > > > >
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > Hi Jun,
>> > > > > > > >
>> > > > > > > > We don't need to bump any RPC requests. The subscription is
>> > > > > serialized
>> > > > > > > > (including its version) and included as bytes in the RPCs.
>> > > > > > > >
>> > > > > > > > Best,
>> > > > > > > > David
>> > > > > > > >
>> > > > > > > > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao
>> > > <jun@confluent.io.invalid
>> > > > >
>> > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > Hi, Rajini,
>> > > > > > > > >
>> > > > > > > > > Thanks for the updated KIP. Just another minor comment. It
>> > > would
>> > > > be
>> > > > > > > > useful
>> > > > > > > > > to list all RPC requests whose version needs to be bumped
>> > > because
>> > > > > of
>> > > > > > > the
>> > > > > > > > > changes in ConsumerProtocolSubscription.
>> > > > > > > > >
>> > > > > > > > > Jun
>> > > > > > > > >
>> > > > > > > > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
>> > > > > > > rajinisivaram@gmail.com>
>> > > > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > > Hi David,
>> > > > > > > > > >
>> > > > > > > > > > Sorry, I was out of office and hence the delay in
>> > responding.
>> > > > > > Thanks
>> > > > > > > > for
>> > > > > > > > > > reviewing the KIP and answering Viktor's question
>> (thanks
>> > for
>> > > > the
>> > > > > > > > review,
>> > > > > > > > > > Viktor).
>> > > > > > > > > >
>> > > > > > > > > > Responses below:
>> > > > > > > > > > 01)  I was in two minds about adding new assignors,
>> because
>> > > as
>> > > > > you
>> > > > > > > > said,
>> > > > > > > > > > user experience is better if assignors used racks when
>> > > > available.
>> > > > > > But
>> > > > > > > > I was
>> > > > > > > > > > a bit concerned about changing the algorithm in existing
>> > > > > > applications
>> > > > > > > > which
>> > > > > > > > > > were already configuring `client.rack`. It felt less
>> risky
>> > to
>> > > > add
>> > > > > > new
>> > > > > > > > > > assignor implementations instead. But we can retain
>> > existing
>> > > > > logic
>> > > > > > if
>> > > > > > > > a)
>> > > > > > > > > > rack information is not available and b) racks have all
>> > > > > partitions.
>> > > > > > > So
>> > > > > > > > the
>> > > > > > > > > > only case where logic will be different is when rack
>> > > > information
>> > > > > is
>> > > > > > > > > > available because consumers chose to use `client.rack`
>> to
>> > > > benefit
>> > > > > > > from
>> > > > > > > > > > improved locality, but racks only have a subset of
>> > > partitions.
>> > > > It
>> > > > > > > seems
>> > > > > > > > > > reasonable to make existing assignors rack-aware in this
>> > case
>> > > > to
>> > > > > > > > improve
>> > > > > > > > > > locality. I have updated the KIP. Will wait and see if
>> > there
>> > > > are
>> > > > > > any
>> > > > > > > > > > objections to this change.
>> > > > > > > > > >
>> > > > > > > > > > 02) Updated 1), so existing assignor classes will be
>> used.
>> > > > > > > > > >
>> > > > > > > > > > 03) Updated the KIP to use version 3, thanks.
>> > > > > > > > > >
>> > > > > > > > > > If there are no concerns or further comments, I will
>> start
>> > > > voting
>> > > > > > > later
>> > > > > > > > > > today.
>> > > > > > > > > >
>> > > > > > > > > > Thank you,
>> > > > > > > > > >
>> > > > > > > > > > Rajini
>> > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
>> > > > > > > <djacot@confluent.io.invalid
>> > > > > > > > >
>> > > > > > > > > > wrote:
>> > > > > > > > > >
>> > > > > > > > > > > Hi Viktor,
>> > > > > > > > > > >
>> > > > > > > > > > > I can actually answer your question. KIP-848 already
>> > > includes
>> > > > > > rack
>> > > > > > > > > > > awareness in the protocol. It is actually the other
>> way
>> > > > around,
>> > > > > > > this
>> > > > > > > > > > > KIP takes the idea from KIP-848 to implement it in the
>> > > > current
>> > > > > > > > > > > protocol in order to realize the benefits sooner. The
>> new
>> > > > > > protocol
>> > > > > > > > > > > will take a while to be implemented.
>> > > > > > > > > > >
>> > > > > > > > > > > Best,
>> > > > > > > > > > > David
>> > > > > > > > > > >
>> > > > > > > > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <
>> > > > > djacot@confluent.io
>> > > > > > >
>> > > > > > > > wrote:
>> > > > > > > > > > > >
>> > > > > > > > > > > > Hi Rajini,
>> > > > > > > > > > > >
>> > > > > > > > > > > > Thanks for the KIP. I have a few questions/comments:
>> > > > > > > > > > > >
>> > > > > > > > > > > > 01. If I understood correctly, the plan is to add
>> new
>> > > > > assignors
>> > > > > > > > which
>> > > > > > > > > > > > are rack aware. Is this right? I wonder if it is a
>> > > > judicious
>> > > > > > > choice
>> > > > > > > > > > > > here. The main drawback is that clients must be
>> > > configured
>> > > > > > > > correctly
>> > > > > > > > > > > > in order to get the benefits. From a user experience
>> > > > > > perspective,
>> > > > > > > > it
>> > > > > > > > > > > > would be much better if we would only require our
>> users
>> > > to
>> > > > > set
>> > > > > > > > > > > > client.rack. However, I understand the argument of
>> > > keeping
>> > > > > the
>> > > > > > > > > > > > existing assignors as-is in order to limit the risk
>> but
>> > > it
>> > > > > also
>> > > > > > > > means
>> > > > > > > > > > > > that we will have to maintain multiple assignors
>> with a
>> > > > > > somewhat
>> > > > > > > > > > > > similar core logic (within a rack). What do you
>> think?
>> > > > > > > > > > > >
>> > > > > > > > > > > > 02. If we proceed with new rack-aware assignors, we
>> > > should
>> > > > > > > mention
>> > > > > > > > > > > > their fully qualified names in the KIP as they will
>> > > become
>> > > > > part
>> > > > > > > of
>> > > > > > > > our
>> > > > > > > > > > > > public interfaces.
>> > > > > > > > > > > >
>> > > > > > > > > > > > 03. KIP-792 has already introduced version 2 of the
>> > > > > > subscription.
>> > > > > > > > The
>> > > > > > > > > > > > KIP is accepted but the PR is not merged yet. This
>> KIP
>> > > > should
>> > > > > > use
>> > > > > > > > > > > > version 3.
>> > > > > > > > > > > >
>> > > > > > > > > > > > Best,
>> > > > > > > > > > > > David
>> > > > > > > > > > > >
>> > > > > > > > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
>> > > > > > > > > > > > <vi...@cloudera.com.invalid> wrote:
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > Hi Rajini,
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > If I understand correctly, the client.rack config
>> > would
>> > > > > stay
>> > > > > > > > > > supported
>> > > > > > > > > > > > > after KIP-848 but does it expand the scope of that
>> > KIP
>> > > > too
>> > > > > > with
>> > > > > > > > this
>> > > > > > > > > > > > > config? I mean that currently you propose
>> > > > > > > > > > ConsumerProtocolSubscription
>> > > > > > > > > > > to
>> > > > > > > > > > > > > be used but this protocol won't be available and
>> we
>> > > need
>> > > > to
>> > > > > > > > transfer
>> > > > > > > > > > > the
>> > > > > > > > > > > > > config to the coordinator via other means. Should
>> > this
>> > > be
>> > > > > > added
>> > > > > > > > to
>> > > > > > > > > > > that KIP?
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > Thanks,
>> > > > > > > > > > > > > Viktor
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
>> > > > > > > > > > rajinisivaram@gmail.com
>> > > > > > > > > > > >
>> > > > > > > > > > > > > wrote:
>> > > > > > > > > > > > >
>> > > > > > > > > > > > > > Hi Jun,
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > Thank you for the review. Yes, we should add
>> rack
>> > id
>> > > to
>> > > > > > > > > > > Subscription, had
>> > > > > > > > > > > > > > missed that part. Updated the KIP, thank you for
>> > > > pointing
>> > > > > > > that
>> > > > > > > > out!
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > Regards,
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > Rajini
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
>> > > > > > > > <ju...@confluent.io.invalid>
>> > > > > > > > > > > wrote:
>> > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > Hi, Rajini,
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > Thanks for the KIP. Just one comment.
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > Should we add rackId to
>> > > > GroupSubscription.Subscription
>> > > > > > for
>> > > > > > > > each
>> > > > > > > > > > > member?
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > Thanks,
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > Jun
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram
>> <
>> > > > > > > > > > > rajinisivaram@gmail.com>
>> > > > > > > > > > > > > > > wrote:
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > Hi all,
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > I have submitted KIP-881 to implement
>> > rack-aware
>> > > > > > > partition
>> > > > > > > > > > > assignment
>> > > > > > > > > > > > > > for
>> > > > > > > > > > > > > > > > consumers:
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
>> > > > > > > > > > > > > > > > .
>> > > > > > > > > > > > > > > > It adds rack id to the consumer group
>> protocol
>> > to
>> > > > > > > propagate
>> > > > > > > > > > rack
>> > > > > > > > > > > > > > > > information so that rack-aware assignors
>> can be
>> > > > added
>> > > > > > to
>> > > > > > > > > > benefit
>> > > > > > > > > > > from
>> > > > > > > > > > > > > > > > locality.
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > Feedback and suggestions are welcome!
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > Thank you,
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > > > Rajini
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Hi Artem,

Yes, clients can reset their `client.rack` config if the broker doesn't use
a rack-aware selector. This config is only useful for improving locality
with a rack-aware selector.

On Wed, Nov 30, 2022 at 10:05 PM Artem Livshits
<al...@confluent.io.invalid> wrote:

> I think it's reasonable for practical scenarios.  Is it possible to turn
> off rack awareness in the clients in case the broker selector plugin is not
> compatible with rack-aware logic in the client?
>
> -Artem
>
> On Wed, Nov 30, 2022 at 2:46 AM Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Artem,
> >
> > Understood your concern - brokers could use a replica selector that uses
> > some other client metadata other than rack id to decide the preferred
> read
> > replica, or just use the leader. In that case, consumers wouldn't really
> > benefit from aligning partition assignment on rack ids. So the question
> is
> > whether we should make the default consumer assignors use rack ids for
> > partition assignment or whether we should have different rack-aware
> > assignors that can be configured when brokers use rack-aware replica
> > selector. We had a similar discussion earlier in the thread (the KIP had
> > originally proposed new rack-aware assignors). We decided to update the
> > default assignors because:
> > 1) Consumers using fetch-from-follower automatically benefit from
> improved
> > locality, without having to update another config.
> > 2) Consumers configure rack id for improved locality, so aligning on
> > replica rack ids generally makes sense.
> > 3) We prioritize balanced assignment over locality in the consumer, so
> the
> > default assignors should work effectively regardless of broker's replica
> > selector.
> >
> > Does that make sense?
> >
> >
> > Thank you,
> >
> > Rajini
> >
> >
> >
> > On Tue, Nov 29, 2022 at 1:05 AM Artem Livshits
> > <al...@confluent.io.invalid> wrote:
> >
> > > I'm thinking of a case where the broker uses a plugin that does not use
> > > rack ids to determine client locality, in that case the consumer might
> > > arrive at the wrong decision based on rack ids.
> > >
> > > -Artem
> > >
> > > On Wed, Nov 23, 2022 at 3:54 AM Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Hi Artem,
> > > >
> > > > Thanks for reviewing the KIP. The client doesn't rely on a particular
> > > > replica assignment logic in the broker. Instead, it matches the
> actual
> > > rack
> > > > assignment for partitions from the metadata with consumer racks. So
> > there
> > > > is no assumption in the client implementation about the use of
> > > > RackAwareReplicaSelector in the broker. Did I misunderstand your
> > concern?
> > > >
> > > > Regards,
> > > >
> > > > Rajini
> > > >
> > > >
> > > > On Tue, Nov 22, 2022 at 11:03 PM Artem Livshits
> > > > <al...@confluent.io.invalid> wrote:
> > > >
> > > > > Hi Rajini,
> > > > >
> > > > > Thank you for the KIP, the KIP looks good to match
> > > > RackAwareReplicaSelector
> > > > > behavior that is available out-of-box.  Which should probably be
> good
> > > > > enough in practice.
> > > > >
> > > > > From the design perspective, though, RackAwareReplicaSelector is
> just
> > > one
> > > > > possible plugin, in theory the broker could use a plugin that
> > leverages
> > > > > networking information to get client locality or some other way, so
> > it
> > > > > seems like we're making an assumption about broker replica
> selection
> > in
> > > > the
> > > > > default assignment implementation.  So I wonder if we should use a
> > > > separate
> > > > > plugin that would be set when RackAwareReplicaSelector is set,
> rather
> > > > than
> > > > > assume broker behavior in the client implementation.
> > > > >
> > > > > -Artem
> > > > >
> > > > > On Wed, Nov 16, 2022 at 8:08 AM Jun Rao <ju...@confluent.io.invalid>
> > > > wrote:
> > > > >
> > > > > > Hi, David and Rajini,
> > > > > >
> > > > > > Thanks for the explanation. It makes sense to me now.
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > > On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <
> > > > rajinisivaram@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Thanks David, that was my understanding as well.
> > > > > > >
> > > > > > > Regards,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > > > On Wed, Nov 16, 2022 at 8:08 AM David Jacot
> > > > > <djacot@confluent.io.invalid
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Jun,
> > > > > > > >
> > > > > > > > We don't need to bump any RPC requests. The subscription is
> > > > > serialized
> > > > > > > > (including its version) and included as bytes in the RPCs.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > David
> > > > > > > >
> > > > > > > > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao
> > > <jun@confluent.io.invalid
> > > > >
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi, Rajini,
> > > > > > > > >
> > > > > > > > > Thanks for the updated KIP. Just another minor comment. It
> > > would
> > > > be
> > > > > > > > useful
> > > > > > > > > to list all RPC requests whose version needs to be bumped
> > > because
> > > > > of
> > > > > > > the
> > > > > > > > > changes in ConsumerProtocolSubscription.
> > > > > > > > >
> > > > > > > > > Jun
> > > > > > > > >
> > > > > > > > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
> > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi David,
> > > > > > > > > >
> > > > > > > > > > Sorry, I was out of office and hence the delay in
> > responding.
> > > > > > Thanks
> > > > > > > > for
> > > > > > > > > > reviewing the KIP and answering Viktor's question (thanks
> > for
> > > > the
> > > > > > > > review,
> > > > > > > > > > Viktor).
> > > > > > > > > >
> > > > > > > > > > Responses below:
> > > > > > > > > > 01)  I was in two minds about adding new assignors,
> because
> > > as
> > > > > you
> > > > > > > > said,
> > > > > > > > > > user experience is better if assignors used racks when
> > > > available.
> > > > > > But
> > > > > > > > I was
> > > > > > > > > > a bit concerned about changing the algorithm in existing
> > > > > > applications
> > > > > > > > which
> > > > > > > > > > were already configuring `client.rack`. It felt less
> risky
> > to
> > > > add
> > > > > > new
> > > > > > > > > > assignor implementations instead. But we can retain
> > existing
> > > > > logic
> > > > > > if
> > > > > > > > a)
> > > > > > > > > > rack information is not available and b) racks have all
> > > > > partitions.
> > > > > > > So
> > > > > > > > the
> > > > > > > > > > only case where logic will be different is when rack
> > > > information
> > > > > is
> > > > > > > > > > available because consumers chose to use `client.rack` to
> > > > benefit
> > > > > > > from
> > > > > > > > > > improved locality, but racks only have a subset of
> > > partitions.
> > > > It
> > > > > > > seems
> > > > > > > > > > reasonable to make existing assignors rack-aware in this
> > case
> > > > to
> > > > > > > > improve
> > > > > > > > > > locality. I have updated the KIP. Will wait and see if
> > there
> > > > are
> > > > > > any
> > > > > > > > > > objections to this change.
> > > > > > > > > >
> > > > > > > > > > 02) Updated 1), so existing assignor classes will be
> used.
> > > > > > > > > >
> > > > > > > > > > 03) Updated the KIP to use version 3, thanks.
> > > > > > > > > >
> > > > > > > > > > If there are no concerns or further comments, I will
> start
> > > > voting
> > > > > > > later
> > > > > > > > > > today.
> > > > > > > > > >
> > > > > > > > > > Thank you,
> > > > > > > > > >
> > > > > > > > > > Rajini
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
> > > > > > > <djacot@confluent.io.invalid
> > > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Viktor,
> > > > > > > > > > >
> > > > > > > > > > > I can actually answer your question. KIP-848 already
> > > includes
> > > > > > rack
> > > > > > > > > > > awareness in the protocol. It is actually the other way
> > > > around,
> > > > > > > this
> > > > > > > > > > > KIP takes the idea from KIP-848 to implement it in the
> > > > current
> > > > > > > > > > > protocol in order to realize the benefits sooner. The
> new
> > > > > > protocol
> > > > > > > > > > > will take a while to be implemented.
> > > > > > > > > > >
> > > > > > > > > > > Best,
> > > > > > > > > > > David
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <
> > > > > djacot@confluent.io
> > > > > > >
> > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Rajini,
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for the KIP. I have a few questions/comments:
> > > > > > > > > > > >
> > > > > > > > > > > > 01. If I understood correctly, the plan is to add new
> > > > > assignors
> > > > > > > > which
> > > > > > > > > > > > are rack aware. Is this right? I wonder if it is a
> > > > judicious
> > > > > > > choice
> > > > > > > > > > > > here. The main drawback is that clients must be
> > > configured
> > > > > > > > correctly
> > > > > > > > > > > > in order to get the benefits. From a user experience
> > > > > > perspective,
> > > > > > > > it
> > > > > > > > > > > > would be much better if we would only require our
> users
> > > to
> > > > > set
> > > > > > > > > > > > client.rack. However, I understand the argument of
> > > keeping
> > > > > the
> > > > > > > > > > > > existing assignors as-is in order to limit the risk
> but
> > > it
> > > > > also
> > > > > > > > means
> > > > > > > > > > > > that we will have to maintain multiple assignors
> with a
> > > > > > somewhat
> > > > > > > > > > > > similar core logic (within a rack). What do you
> think?
> > > > > > > > > > > >
> > > > > > > > > > > > 02. If we proceed with new rack-aware assignors, we
> > > should
> > > > > > > mention
> > > > > > > > > > > > their fully qualified names in the KIP as they will
> > > become
> > > > > part
> > > > > > > of
> > > > > > > > our
> > > > > > > > > > > > public interfaces.
> > > > > > > > > > > >
> > > > > > > > > > > > 03. KIP-792 has already introduced version 2 of the
> > > > > > subscription.
> > > > > > > > The
> > > > > > > > > > > > KIP is accepted but the PR is not merged yet. This
> KIP
> > > > should
> > > > > > use
> > > > > > > > > > > > version 3.
> > > > > > > > > > > >
> > > > > > > > > > > > Best,
> > > > > > > > > > > > David
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > > > > > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Rajini,
> > > > > > > > > > > > >
> > > > > > > > > > > > > If I understand correctly, the client.rack config
> > would
> > > > > stay
> > > > > > > > > > supported
> > > > > > > > > > > > > after KIP-848 but does it expand the scope of that
> > KIP
> > > > too
> > > > > > with
> > > > > > > > this
> > > > > > > > > > > > > config? I mean that currently you propose
> > > > > > > > > > ConsumerProtocolSubscription
> > > > > > > > > > > to
> > > > > > > > > > > > > be used but this protocol won't be available and we
> > > need
> > > > to
> > > > > > > > transfer
> > > > > > > > > > > the
> > > > > > > > > > > > > config to the coordinator via other means. Should
> > this
> > > be
> > > > > > added
> > > > > > > > to
> > > > > > > > > > > that KIP?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Viktor
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > > > > > > > > rajinisivaram@gmail.com
> > > > > > > > > > > >
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Jun,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thank you for the review. Yes, we should add rack
> > id
> > > to
> > > > > > > > > > > Subscription, had
> > > > > > > > > > > > > > missed that part. Updated the KIP, thank you for
> > > > pointing
> > > > > > > that
> > > > > > > > out!
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Regards,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Rajini
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> > > > > > > > <ju...@confluent.io.invalid>
> > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi, Rajini,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Should we add rackId to
> > > > GroupSubscription.Subscription
> > > > > > for
> > > > > > > > each
> > > > > > > > > > > member?
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Jun
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > > > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi all,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I have submitted KIP-881 to implement
> > rack-aware
> > > > > > > partition
> > > > > > > > > > > assignment
> > > > > > > > > > > > > > for
> > > > > > > > > > > > > > > > consumers:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > > > > > > > > .
> > > > > > > > > > > > > > > > It adds rack id to the consumer group
> protocol
> > to
> > > > > > > propagate
> > > > > > > > > > rack
> > > > > > > > > > > > > > > > information so that rack-aware assignors can
> be
> > > > added
> > > > > > to
> > > > > > > > > > benefit
> > > > > > > > > > > from
> > > > > > > > > > > > > > > > locality.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Thank you,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Rajini
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Artem Livshits <al...@confluent.io.INVALID>]

I think it's reasonable for practical scenarios.  Is it possible to turn
off rack awareness in the clients in case the broker selector plugin is not
compatible with rack-aware logic in the client?

-Artem

On Wed, Nov 30, 2022 at 2:46 AM Rajini Sivaram <ra...@gmail.com>
wrote:

> Hi Artem,
>
> Understood your concern - brokers could use a replica selector that uses
> some other client metadata other than rack id to decide the preferred read
> replica, or just use the leader. In that case, consumers wouldn't really
> benefit from aligning partition assignment on rack ids. So the question is
> whether we should make the default consumer assignors use rack ids for
> partition assignment or whether we should have different rack-aware
> assignors that can be configured when brokers use rack-aware replica
> selector. We had a similar discussion earlier in the thread (the KIP had
> originally proposed new rack-aware assignors). We decided to update the
> default assignors because:
> 1) Consumers using fetch-from-follower automatically benefit from improved
> locality, without having to update another config.
> 2) Consumers configure rack id for improved locality, so aligning on
> replica rack ids generally makes sense.
> 3) We prioritize balanced assignment over locality in the consumer, so the
> default assignors should work effectively regardless of broker's replica
> selector.
>
> Does that make sense?
>
>
> Thank you,
>
> Rajini
>
>
>
> On Tue, Nov 29, 2022 at 1:05 AM Artem Livshits
> <al...@confluent.io.invalid> wrote:
>
> > I'm thinking of a case where the broker uses a plugin that does not use
> > rack ids to determine client locality, in that case the consumer might
> > arrive at the wrong decision based on rack ids.
> >
> > -Artem
> >
> > On Wed, Nov 23, 2022 at 3:54 AM Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi Artem,
> > >
> > > Thanks for reviewing the KIP. The client doesn't rely on a particular
> > > replica assignment logic in the broker. Instead, it matches the actual
> > rack
> > > assignment for partitions from the metadata with consumer racks. So
> there
> > > is no assumption in the client implementation about the use of
> > > RackAwareReplicaSelector in the broker. Did I misunderstand your
> concern?
> > >
> > > Regards,
> > >
> > > Rajini
> > >
> > >
> > > On Tue, Nov 22, 2022 at 11:03 PM Artem Livshits
> > > <al...@confluent.io.invalid> wrote:
> > >
> > > > Hi Rajini,
> > > >
> > > > Thank you for the KIP, the KIP looks good to match
> > > RackAwareReplicaSelector
> > > > behavior that is available out-of-box.  Which should probably be good
> > > > enough in practice.
> > > >
> > > > From the design perspective, though, RackAwareReplicaSelector is just
> > one
> > > > possible plugin, in theory the broker could use a plugin that
> leverages
> > > > networking information to get client locality or some other way, so
> it
> > > > seems like we're making an assumption about broker replica selection
> in
> > > the
> > > > default assignment implementation.  So I wonder if we should use a
> > > separate
> > > > plugin that would be set when RackAwareReplicaSelector is set, rather
> > > than
> > > > assume broker behavior in the client implementation.
> > > >
> > > > -Artem
> > > >
> > > > On Wed, Nov 16, 2022 at 8:08 AM Jun Rao <ju...@confluent.io.invalid>
> > > wrote:
> > > >
> > > > > Hi, David and Rajini,
> > > > >
> > > > > Thanks for the explanation. It makes sense to me now.
> > > > >
> > > > > Jun
> > > > >
> > > > > On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Thanks David, that was my understanding as well.
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > > On Wed, Nov 16, 2022 at 8:08 AM David Jacot
> > > > <djacot@confluent.io.invalid
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Jun,
> > > > > > >
> > > > > > > We don't need to bump any RPC requests. The subscription is
> > > > serialized
> > > > > > > (including its version) and included as bytes in the RPCs.
> > > > > > >
> > > > > > > Best,
> > > > > > > David
> > > > > > >
> > > > > > > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao
> > <jun@confluent.io.invalid
> > > >
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi, Rajini,
> > > > > > > >
> > > > > > > > Thanks for the updated KIP. Just another minor comment. It
> > would
> > > be
> > > > > > > useful
> > > > > > > > to list all RPC requests whose version needs to be bumped
> > because
> > > > of
> > > > > > the
> > > > > > > > changes in ConsumerProtocolSubscription.
> > > > > > > >
> > > > > > > > Jun
> > > > > > > >
> > > > > > > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
> > > > > > rajinisivaram@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi David,
> > > > > > > > >
> > > > > > > > > Sorry, I was out of office and hence the delay in
> responding.
> > > > > Thanks
> > > > > > > for
> > > > > > > > > reviewing the KIP and answering Viktor's question (thanks
> for
> > > the
> > > > > > > review,
> > > > > > > > > Viktor).
> > > > > > > > >
> > > > > > > > > Responses below:
> > > > > > > > > 01)  I was in two minds about adding new assignors, because
> > as
> > > > you
> > > > > > > said,
> > > > > > > > > user experience is better if assignors used racks when
> > > available.
> > > > > But
> > > > > > > I was
> > > > > > > > > a bit concerned about changing the algorithm in existing
> > > > > applications
> > > > > > > which
> > > > > > > > > were already configuring `client.rack`. It felt less risky
> to
> > > add
> > > > > new
> > > > > > > > > assignor implementations instead. But we can retain
> existing
> > > > logic
> > > > > if
> > > > > > > a)
> > > > > > > > > rack information is not available and b) racks have all
> > > > partitions.
> > > > > > So
> > > > > > > the
> > > > > > > > > only case where logic will be different is when rack
> > > information
> > > > is
> > > > > > > > > available because consumers chose to use `client.rack` to
> > > benefit
> > > > > > from
> > > > > > > > > improved locality, but racks only have a subset of
> > partitions.
> > > It
> > > > > > seems
> > > > > > > > > reasonable to make existing assignors rack-aware in this
> case
> > > to
> > > > > > > improve
> > > > > > > > > locality. I have updated the KIP. Will wait and see if
> there
> > > are
> > > > > any
> > > > > > > > > objections to this change.
> > > > > > > > >
> > > > > > > > > 02) Updated 1), so existing assignor classes will be used.
> > > > > > > > >
> > > > > > > > > 03) Updated the KIP to use version 3, thanks.
> > > > > > > > >
> > > > > > > > > If there are no concerns or further comments, I will start
> > > voting
> > > > > > later
> > > > > > > > > today.
> > > > > > > > >
> > > > > > > > > Thank you,
> > > > > > > > >
> > > > > > > > > Rajini
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
> > > > > > <djacot@confluent.io.invalid
> > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Viktor,
> > > > > > > > > >
> > > > > > > > > > I can actually answer your question. KIP-848 already
> > includes
> > > > > rack
> > > > > > > > > > awareness in the protocol. It is actually the other way
> > > around,
> > > > > > this
> > > > > > > > > > KIP takes the idea from KIP-848 to implement it in the
> > > current
> > > > > > > > > > protocol in order to realize the benefits sooner. The new
> > > > > protocol
> > > > > > > > > > will take a while to be implemented.
> > > > > > > > > >
> > > > > > > > > > Best,
> > > > > > > > > > David
> > > > > > > > > >
> > > > > > > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <
> > > > djacot@confluent.io
> > > > > >
> > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Rajini,
> > > > > > > > > > >
> > > > > > > > > > > Thanks for the KIP. I have a few questions/comments:
> > > > > > > > > > >
> > > > > > > > > > > 01. If I understood correctly, the plan is to add new
> > > > assignors
> > > > > > > which
> > > > > > > > > > > are rack aware. Is this right? I wonder if it is a
> > > judicious
> > > > > > choice
> > > > > > > > > > > here. The main drawback is that clients must be
> > configured
> > > > > > > correctly
> > > > > > > > > > > in order to get the benefits. From a user experience
> > > > > perspective,
> > > > > > > it
> > > > > > > > > > > would be much better if we would only require our users
> > to
> > > > set
> > > > > > > > > > > client.rack. However, I understand the argument of
> > keeping
> > > > the
> > > > > > > > > > > existing assignors as-is in order to limit the risk but
> > it
> > > > also
> > > > > > > means
> > > > > > > > > > > that we will have to maintain multiple assignors with a
> > > > > somewhat
> > > > > > > > > > > similar core logic (within a rack). What do you think?
> > > > > > > > > > >
> > > > > > > > > > > 02. If we proceed with new rack-aware assignors, we
> > should
> > > > > > mention
> > > > > > > > > > > their fully qualified names in the KIP as they will
> > become
> > > > part
> > > > > > of
> > > > > > > our
> > > > > > > > > > > public interfaces.
> > > > > > > > > > >
> > > > > > > > > > > 03. KIP-792 has already introduced version 2 of the
> > > > > subscription.
> > > > > > > The
> > > > > > > > > > > KIP is accepted but the PR is not merged yet. This KIP
> > > should
> > > > > use
> > > > > > > > > > > version 3.
> > > > > > > > > > >
> > > > > > > > > > > Best,
> > > > > > > > > > > David
> > > > > > > > > > >
> > > > > > > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > > > > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Rajini,
> > > > > > > > > > > >
> > > > > > > > > > > > If I understand correctly, the client.rack config
> would
> > > > stay
> > > > > > > > > supported
> > > > > > > > > > > > after KIP-848 but does it expand the scope of that
> KIP
> > > too
> > > > > with
> > > > > > > this
> > > > > > > > > > > > config? I mean that currently you propose
> > > > > > > > > ConsumerProtocolSubscription
> > > > > > > > > > to
> > > > > > > > > > > > be used but this protocol won't be available and we
> > need
> > > to
> > > > > > > transfer
> > > > > > > > > > the
> > > > > > > > > > > > config to the coordinator via other means. Should
> this
> > be
> > > > > added
> > > > > > > to
> > > > > > > > > > that KIP?
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Viktor
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > > > > > > > rajinisivaram@gmail.com
> > > > > > > > > > >
> > > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hi Jun,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thank you for the review. Yes, we should add rack
> id
> > to
> > > > > > > > > > Subscription, had
> > > > > > > > > > > > > missed that part. Updated the KIP, thank you for
> > > pointing
> > > > > > that
> > > > > > > out!
> > > > > > > > > > > > >
> > > > > > > > > > > > > Regards,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Rajini
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> > > > > > > <ju...@confluent.io.invalid>
> > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi, Rajini,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Should we add rackId to
> > > GroupSubscription.Subscription
> > > > > for
> > > > > > > each
> > > > > > > > > > member?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Jun
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi all,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I have submitted KIP-881 to implement
> rack-aware
> > > > > > partition
> > > > > > > > > > assignment
> > > > > > > > > > > > > for
> > > > > > > > > > > > > > > consumers:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > > > > > > > .
> > > > > > > > > > > > > > > It adds rack id to the consumer group protocol
> to
> > > > > > propagate
> > > > > > > > > rack
> > > > > > > > > > > > > > > information so that rack-aware assignors can be
> > > added
> > > > > to
> > > > > > > > > benefit
> > > > > > > > > > from
> > > > > > > > > > > > > > > locality.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thank you,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Rajini
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Hi Artem,

Understood your concern - brokers could use a replica selector that uses
some other client metadata other than rack id to decide the preferred read
replica, or just use the leader. In that case, consumers wouldn't really
benefit from aligning partition assignment on rack ids. So the question is
whether we should make the default consumer assignors use rack ids for
partition assignment or whether we should have different rack-aware
assignors that can be configured when brokers use rack-aware replica
selector. We had a similar discussion earlier in the thread (the KIP had
originally proposed new rack-aware assignors). We decided to update the
default assignors because:
1) Consumers using fetch-from-follower automatically benefit from improved
locality, without having to update another config.
2) Consumers configure rack id for improved locality, so aligning on
replica rack ids generally makes sense.
3) We prioritize balanced assignment over locality in the consumer, so the
default assignors should work effectively regardless of broker's replica
selector.

Does that make sense?


Thank you,

Rajini



On Tue, Nov 29, 2022 at 1:05 AM Artem Livshits
<al...@confluent.io.invalid> wrote:

> I'm thinking of a case where the broker uses a plugin that does not use
> rack ids to determine client locality, in that case the consumer might
> arrive at the wrong decision based on rack ids.
>
> -Artem
>
> On Wed, Nov 23, 2022 at 3:54 AM Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Artem,
> >
> > Thanks for reviewing the KIP. The client doesn't rely on a particular
> > replica assignment logic in the broker. Instead, it matches the actual
> rack
> > assignment for partitions from the metadata with consumer racks. So there
> > is no assumption in the client implementation about the use of
> > RackAwareReplicaSelector in the broker. Did I misunderstand your concern?
> >
> > Regards,
> >
> > Rajini
> >
> >
> > On Tue, Nov 22, 2022 at 11:03 PM Artem Livshits
> > <al...@confluent.io.invalid> wrote:
> >
> > > Hi Rajini,
> > >
> > > Thank you for the KIP, the KIP looks good to match
> > RackAwareReplicaSelector
> > > behavior that is available out-of-box.  Which should probably be good
> > > enough in practice.
> > >
> > > From the design perspective, though, RackAwareReplicaSelector is just
> one
> > > possible plugin, in theory the broker could use a plugin that leverages
> > > networking information to get client locality or some other way, so it
> > > seems like we're making an assumption about broker replica selection in
> > the
> > > default assignment implementation.  So I wonder if we should use a
> > separate
> > > plugin that would be set when RackAwareReplicaSelector is set, rather
> > than
> > > assume broker behavior in the client implementation.
> > >
> > > -Artem
> > >
> > > On Wed, Nov 16, 2022 at 8:08 AM Jun Rao <ju...@confluent.io.invalid>
> > wrote:
> > >
> > > > Hi, David and Rajini,
> > > >
> > > > Thanks for the explanation. It makes sense to me now.
> > > >
> > > > Jun
> > > >
> > > > On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > wrote:
> > > >
> > > > > Thanks David, that was my understanding as well.
> > > > >
> > > > > Regards,
> > > > >
> > > > > Rajini
> > > > >
> > > > > On Wed, Nov 16, 2022 at 8:08 AM David Jacot
> > > <djacot@confluent.io.invalid
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Jun,
> > > > > >
> > > > > > We don't need to bump any RPC requests. The subscription is
> > > serialized
> > > > > > (including its version) and included as bytes in the RPCs.
> > > > > >
> > > > > > Best,
> > > > > > David
> > > > > >
> > > > > > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao
> <jun@confluent.io.invalid
> > >
> > > > > wrote:
> > > > > > >
> > > > > > > Hi, Rajini,
> > > > > > >
> > > > > > > Thanks for the updated KIP. Just another minor comment. It
> would
> > be
> > > > > > useful
> > > > > > > to list all RPC requests whose version needs to be bumped
> because
> > > of
> > > > > the
> > > > > > > changes in ConsumerProtocolSubscription.
> > > > > > >
> > > > > > > Jun
> > > > > > >
> > > > > > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
> > > > > rajinisivaram@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi David,
> > > > > > > >
> > > > > > > > Sorry, I was out of office and hence the delay in responding.
> > > > Thanks
> > > > > > for
> > > > > > > > reviewing the KIP and answering Viktor's question (thanks for
> > the
> > > > > > review,
> > > > > > > > Viktor).
> > > > > > > >
> > > > > > > > Responses below:
> > > > > > > > 01)  I was in two minds about adding new assignors, because
> as
> > > you
> > > > > > said,
> > > > > > > > user experience is better if assignors used racks when
> > available.
> > > > But
> > > > > > I was
> > > > > > > > a bit concerned about changing the algorithm in existing
> > > > applications
> > > > > > which
> > > > > > > > were already configuring `client.rack`. It felt less risky to
> > add
> > > > new
> > > > > > > > assignor implementations instead. But we can retain existing
> > > logic
> > > > if
> > > > > > a)
> > > > > > > > rack information is not available and b) racks have all
> > > partitions.
> > > > > So
> > > > > > the
> > > > > > > > only case where logic will be different is when rack
> > information
> > > is
> > > > > > > > available because consumers chose to use `client.rack` to
> > benefit
> > > > > from
> > > > > > > > improved locality, but racks only have a subset of
> partitions.
> > It
> > > > > seems
> > > > > > > > reasonable to make existing assignors rack-aware in this case
> > to
> > > > > > improve
> > > > > > > > locality. I have updated the KIP. Will wait and see if there
> > are
> > > > any
> > > > > > > > objections to this change.
> > > > > > > >
> > > > > > > > 02) Updated 1), so existing assignor classes will be used.
> > > > > > > >
> > > > > > > > 03) Updated the KIP to use version 3, thanks.
> > > > > > > >
> > > > > > > > If there are no concerns or further comments, I will start
> > voting
> > > > > later
> > > > > > > > today.
> > > > > > > >
> > > > > > > > Thank you,
> > > > > > > >
> > > > > > > > Rajini
> > > > > > > >
> > > > > > > >
> > > > > > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
> > > > > <djacot@confluent.io.invalid
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Viktor,
> > > > > > > > >
> > > > > > > > > I can actually answer your question. KIP-848 already
> includes
> > > > rack
> > > > > > > > > awareness in the protocol. It is actually the other way
> > around,
> > > > > this
> > > > > > > > > KIP takes the idea from KIP-848 to implement it in the
> > current
> > > > > > > > > protocol in order to realize the benefits sooner. The new
> > > > protocol
> > > > > > > > > will take a while to be implemented.
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > David
> > > > > > > > >
> > > > > > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <
> > > djacot@confluent.io
> > > > >
> > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Rajini,
> > > > > > > > > >
> > > > > > > > > > Thanks for the KIP. I have a few questions/comments:
> > > > > > > > > >
> > > > > > > > > > 01. If I understood correctly, the plan is to add new
> > > assignors
> > > > > > which
> > > > > > > > > > are rack aware. Is this right? I wonder if it is a
> > judicious
> > > > > choice
> > > > > > > > > > here. The main drawback is that clients must be
> configured
> > > > > > correctly
> > > > > > > > > > in order to get the benefits. From a user experience
> > > > perspective,
> > > > > > it
> > > > > > > > > > would be much better if we would only require our users
> to
> > > set
> > > > > > > > > > client.rack. However, I understand the argument of
> keeping
> > > the
> > > > > > > > > > existing assignors as-is in order to limit the risk but
> it
> > > also
> > > > > > means
> > > > > > > > > > that we will have to maintain multiple assignors with a
> > > > somewhat
> > > > > > > > > > similar core logic (within a rack). What do you think?
> > > > > > > > > >
> > > > > > > > > > 02. If we proceed with new rack-aware assignors, we
> should
> > > > > mention
> > > > > > > > > > their fully qualified names in the KIP as they will
> become
> > > part
> > > > > of
> > > > > > our
> > > > > > > > > > public interfaces.
> > > > > > > > > >
> > > > > > > > > > 03. KIP-792 has already introduced version 2 of the
> > > > subscription.
> > > > > > The
> > > > > > > > > > KIP is accepted but the PR is not merged yet. This KIP
> > should
> > > > use
> > > > > > > > > > version 3.
> > > > > > > > > >
> > > > > > > > > > Best,
> > > > > > > > > > David
> > > > > > > > > >
> > > > > > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > > > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Rajini,
> > > > > > > > > > >
> > > > > > > > > > > If I understand correctly, the client.rack config would
> > > stay
> > > > > > > > supported
> > > > > > > > > > > after KIP-848 but does it expand the scope of that KIP
> > too
> > > > with
> > > > > > this
> > > > > > > > > > > config? I mean that currently you propose
> > > > > > > > ConsumerProtocolSubscription
> > > > > > > > > to
> > > > > > > > > > > be used but this protocol won't be available and we
> need
> > to
> > > > > > transfer
> > > > > > > > > the
> > > > > > > > > > > config to the coordinator via other means. Should this
> be
> > > > added
> > > > > > to
> > > > > > > > > that KIP?
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > > Viktor
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > > > > > > rajinisivaram@gmail.com
> > > > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Jun,
> > > > > > > > > > > >
> > > > > > > > > > > > Thank you for the review. Yes, we should add rack id
> to
> > > > > > > > > Subscription, had
> > > > > > > > > > > > missed that part. Updated the KIP, thank you for
> > pointing
> > > > > that
> > > > > > out!
> > > > > > > > > > > >
> > > > > > > > > > > > Regards,
> > > > > > > > > > > >
> > > > > > > > > > > > Rajini
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> > > > > > <ju...@confluent.io.invalid>
> > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hi, Rajini,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Should we add rackId to
> > GroupSubscription.Subscription
> > > > for
> > > > > > each
> > > > > > > > > member?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Jun
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi all,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I have submitted KIP-881 to implement rack-aware
> > > > > partition
> > > > > > > > > assignment
> > > > > > > > > > > > for
> > > > > > > > > > > > > > consumers:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > > > > > > .
> > > > > > > > > > > > > > It adds rack id to the consumer group protocol to
> > > > > propagate
> > > > > > > > rack
> > > > > > > > > > > > > > information so that rack-aware assignors can be
> > added
> > > > to
> > > > > > > > benefit
> > > > > > > > > from
> > > > > > > > > > > > > > locality.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thank you,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Rajini
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Artem Livshits <al...@confluent.io.INVALID>]

I'm thinking of a case where the broker uses a plugin that does not use
rack ids to determine client locality, in that case the consumer might
arrive at the wrong decision based on rack ids.

-Artem

On Wed, Nov 23, 2022 at 3:54 AM Rajini Sivaram <ra...@gmail.com>
wrote:

> Hi Artem,
>
> Thanks for reviewing the KIP. The client doesn't rely on a particular
> replica assignment logic in the broker. Instead, it matches the actual rack
> assignment for partitions from the metadata with consumer racks. So there
> is no assumption in the client implementation about the use of
> RackAwareReplicaSelector in the broker. Did I misunderstand your concern?
>
> Regards,
>
> Rajini
>
>
> On Tue, Nov 22, 2022 at 11:03 PM Artem Livshits
> <al...@confluent.io.invalid> wrote:
>
> > Hi Rajini,
> >
> > Thank you for the KIP, the KIP looks good to match
> RackAwareReplicaSelector
> > behavior that is available out-of-box.  Which should probably be good
> > enough in practice.
> >
> > From the design perspective, though, RackAwareReplicaSelector is just one
> > possible plugin, in theory the broker could use a plugin that leverages
> > networking information to get client locality or some other way, so it
> > seems like we're making an assumption about broker replica selection in
> the
> > default assignment implementation.  So I wonder if we should use a
> separate
> > plugin that would be set when RackAwareReplicaSelector is set, rather
> than
> > assume broker behavior in the client implementation.
> >
> > -Artem
> >
> > On Wed, Nov 16, 2022 at 8:08 AM Jun Rao <ju...@confluent.io.invalid>
> wrote:
> >
> > > Hi, David and Rajini,
> > >
> > > Thanks for the explanation. It makes sense to me now.
> > >
> > > Jun
> > >
> > > On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Thanks David, that was my understanding as well.
> > > >
> > > > Regards,
> > > >
> > > > Rajini
> > > >
> > > > On Wed, Nov 16, 2022 at 8:08 AM David Jacot
> > <djacot@confluent.io.invalid
> > > >
> > > > wrote:
> > > >
> > > > > Hi Jun,
> > > > >
> > > > > We don't need to bump any RPC requests. The subscription is
> > serialized
> > > > > (including its version) and included as bytes in the RPCs.
> > > > >
> > > > > Best,
> > > > > David
> > > > >
> > > > > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao <jun@confluent.io.invalid
> >
> > > > wrote:
> > > > > >
> > > > > > Hi, Rajini,
> > > > > >
> > > > > > Thanks for the updated KIP. Just another minor comment. It would
> be
> > > > > useful
> > > > > > to list all RPC requests whose version needs to be bumped because
> > of
> > > > the
> > > > > > changes in ConsumerProtocolSubscription.
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
> > > > rajinisivaram@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi David,
> > > > > > >
> > > > > > > Sorry, I was out of office and hence the delay in responding.
> > > Thanks
> > > > > for
> > > > > > > reviewing the KIP and answering Viktor's question (thanks for
> the
> > > > > review,
> > > > > > > Viktor).
> > > > > > >
> > > > > > > Responses below:
> > > > > > > 01)  I was in two minds about adding new assignors, because as
> > you
> > > > > said,
> > > > > > > user experience is better if assignors used racks when
> available.
> > > But
> > > > > I was
> > > > > > > a bit concerned about changing the algorithm in existing
> > > applications
> > > > > which
> > > > > > > were already configuring `client.rack`. It felt less risky to
> add
> > > new
> > > > > > > assignor implementations instead. But we can retain existing
> > logic
> > > if
> > > > > a)
> > > > > > > rack information is not available and b) racks have all
> > partitions.
> > > > So
> > > > > the
> > > > > > > only case where logic will be different is when rack
> information
> > is
> > > > > > > available because consumers chose to use `client.rack` to
> benefit
> > > > from
> > > > > > > improved locality, but racks only have a subset of partitions.
> It
> > > > seems
> > > > > > > reasonable to make existing assignors rack-aware in this case
> to
> > > > > improve
> > > > > > > locality. I have updated the KIP. Will wait and see if there
> are
> > > any
> > > > > > > objections to this change.
> > > > > > >
> > > > > > > 02) Updated 1), so existing assignor classes will be used.
> > > > > > >
> > > > > > > 03) Updated the KIP to use version 3, thanks.
> > > > > > >
> > > > > > > If there are no concerns or further comments, I will start
> voting
> > > > later
> > > > > > > today.
> > > > > > >
> > > > > > > Thank you,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > > >
> > > > > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
> > > > <djacot@confluent.io.invalid
> > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Viktor,
> > > > > > > >
> > > > > > > > I can actually answer your question. KIP-848 already includes
> > > rack
> > > > > > > > awareness in the protocol. It is actually the other way
> around,
> > > > this
> > > > > > > > KIP takes the idea from KIP-848 to implement it in the
> current
> > > > > > > > protocol in order to realize the benefits sooner. The new
> > > protocol
> > > > > > > > will take a while to be implemented.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > David
> > > > > > > >
> > > > > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <
> > djacot@confluent.io
> > > >
> > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Rajini,
> > > > > > > > >
> > > > > > > > > Thanks for the KIP. I have a few questions/comments:
> > > > > > > > >
> > > > > > > > > 01. If I understood correctly, the plan is to add new
> > assignors
> > > > > which
> > > > > > > > > are rack aware. Is this right? I wonder if it is a
> judicious
> > > > choice
> > > > > > > > > here. The main drawback is that clients must be configured
> > > > > correctly
> > > > > > > > > in order to get the benefits. From a user experience
> > > perspective,
> > > > > it
> > > > > > > > > would be much better if we would only require our users to
> > set
> > > > > > > > > client.rack. However, I understand the argument of keeping
> > the
> > > > > > > > > existing assignors as-is in order to limit the risk but it
> > also
> > > > > means
> > > > > > > > > that we will have to maintain multiple assignors with a
> > > somewhat
> > > > > > > > > similar core logic (within a rack). What do you think?
> > > > > > > > >
> > > > > > > > > 02. If we proceed with new rack-aware assignors, we should
> > > > mention
> > > > > > > > > their fully qualified names in the KIP as they will become
> > part
> > > > of
> > > > > our
> > > > > > > > > public interfaces.
> > > > > > > > >
> > > > > > > > > 03. KIP-792 has already introduced version 2 of the
> > > subscription.
> > > > > The
> > > > > > > > > KIP is accepted but the PR is not merged yet. This KIP
> should
> > > use
> > > > > > > > > version 3.
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > David
> > > > > > > > >
> > > > > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Rajini,
> > > > > > > > > >
> > > > > > > > > > If I understand correctly, the client.rack config would
> > stay
> > > > > > > supported
> > > > > > > > > > after KIP-848 but does it expand the scope of that KIP
> too
> > > with
> > > > > this
> > > > > > > > > > config? I mean that currently you propose
> > > > > > > ConsumerProtocolSubscription
> > > > > > > > to
> > > > > > > > > > be used but this protocol won't be available and we need
> to
> > > > > transfer
> > > > > > > > the
> > > > > > > > > > config to the coordinator via other means. Should this be
> > > added
> > > > > to
> > > > > > > > that KIP?
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > > Viktor
> > > > > > > > > >
> > > > > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > > > > > rajinisivaram@gmail.com
> > > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi Jun,
> > > > > > > > > > >
> > > > > > > > > > > Thank you for the review. Yes, we should add rack id to
> > > > > > > > Subscription, had
> > > > > > > > > > > missed that part. Updated the KIP, thank you for
> pointing
> > > > that
> > > > > out!
> > > > > > > > > > >
> > > > > > > > > > > Regards,
> > > > > > > > > > >
> > > > > > > > > > > Rajini
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> > > > > <ju...@confluent.io.invalid>
> > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi, Rajini,
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > > > > > >
> > > > > > > > > > > > Should we add rackId to
> GroupSubscription.Subscription
> > > for
> > > > > each
> > > > > > > > member?
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > >
> > > > > > > > > > > > Jun
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hi all,
> > > > > > > > > > > > >
> > > > > > > > > > > > > I have submitted KIP-881 to implement rack-aware
> > > > partition
> > > > > > > > assignment
> > > > > > > > > > > for
> > > > > > > > > > > > > consumers:
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > > > > > .
> > > > > > > > > > > > > It adds rack id to the consumer group protocol to
> > > > propagate
> > > > > > > rack
> > > > > > > > > > > > > information so that rack-aware assignors can be
> added
> > > to
> > > > > > > benefit
> > > > > > > > from
> > > > > > > > > > > > > locality.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thank you,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Rajini
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Hi Artem,

Thanks for reviewing the KIP. The client doesn't rely on a particular
replica assignment logic in the broker. Instead, it matches the actual rack
assignment for partitions from the metadata with consumer racks. So there
is no assumption in the client implementation about the use of
RackAwareReplicaSelector in the broker. Did I misunderstand your concern?

Regards,

Rajini


On Tue, Nov 22, 2022 at 11:03 PM Artem Livshits
<al...@confluent.io.invalid> wrote:

> Hi Rajini,
>
> Thank you for the KIP, the KIP looks good to match RackAwareReplicaSelector
> behavior that is available out-of-box.  Which should probably be good
> enough in practice.
>
> From the design perspective, though, RackAwareReplicaSelector is just one
> possible plugin, in theory the broker could use a plugin that leverages
> networking information to get client locality or some other way, so it
> seems like we're making an assumption about broker replica selection in the
> default assignment implementation.  So I wonder if we should use a separate
> plugin that would be set when RackAwareReplicaSelector is set, rather than
> assume broker behavior in the client implementation.
>
> -Artem
>
> On Wed, Nov 16, 2022 at 8:08 AM Jun Rao <ju...@confluent.io.invalid> wrote:
>
> > Hi, David and Rajini,
> >
> > Thanks for the explanation. It makes sense to me now.
> >
> > Jun
> >
> > On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Thanks David, that was my understanding as well.
> > >
> > > Regards,
> > >
> > > Rajini
> > >
> > > On Wed, Nov 16, 2022 at 8:08 AM David Jacot
> <djacot@confluent.io.invalid
> > >
> > > wrote:
> > >
> > > > Hi Jun,
> > > >
> > > > We don't need to bump any RPC requests. The subscription is
> serialized
> > > > (including its version) and included as bytes in the RPCs.
> > > >
> > > > Best,
> > > > David
> > > >
> > > > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao <ju...@confluent.io.invalid>
> > > wrote:
> > > > >
> > > > > Hi, Rajini,
> > > > >
> > > > > Thanks for the updated KIP. Just another minor comment. It would be
> > > > useful
> > > > > to list all RPC requests whose version needs to be bumped because
> of
> > > the
> > > > > changes in ConsumerProtocolSubscription.
> > > > >
> > > > > Jun
> > > > >
> > > > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi David,
> > > > > >
> > > > > > Sorry, I was out of office and hence the delay in responding.
> > Thanks
> > > > for
> > > > > > reviewing the KIP and answering Viktor's question (thanks for the
> > > > review,
> > > > > > Viktor).
> > > > > >
> > > > > > Responses below:
> > > > > > 01)  I was in two minds about adding new assignors, because as
> you
> > > > said,
> > > > > > user experience is better if assignors used racks when available.
> > But
> > > > I was
> > > > > > a bit concerned about changing the algorithm in existing
> > applications
> > > > which
> > > > > > were already configuring `client.rack`. It felt less risky to add
> > new
> > > > > > assignor implementations instead. But we can retain existing
> logic
> > if
> > > > a)
> > > > > > rack information is not available and b) racks have all
> partitions.
> > > So
> > > > the
> > > > > > only case where logic will be different is when rack information
> is
> > > > > > available because consumers chose to use `client.rack` to benefit
> > > from
> > > > > > improved locality, but racks only have a subset of partitions. It
> > > seems
> > > > > > reasonable to make existing assignors rack-aware in this case to
> > > > improve
> > > > > > locality. I have updated the KIP. Will wait and see if there are
> > any
> > > > > > objections to this change.
> > > > > >
> > > > > > 02) Updated 1), so existing assignor classes will be used.
> > > > > >
> > > > > > 03) Updated the KIP to use version 3, thanks.
> > > > > >
> > > > > > If there are no concerns or further comments, I will start voting
> > > later
> > > > > > today.
> > > > > >
> > > > > > Thank you,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > >
> > > > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
> > > <djacot@confluent.io.invalid
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Viktor,
> > > > > > >
> > > > > > > I can actually answer your question. KIP-848 already includes
> > rack
> > > > > > > awareness in the protocol. It is actually the other way around,
> > > this
> > > > > > > KIP takes the idea from KIP-848 to implement it in the current
> > > > > > > protocol in order to realize the benefits sooner. The new
> > protocol
> > > > > > > will take a while to be implemented.
> > > > > > >
> > > > > > > Best,
> > > > > > > David
> > > > > > >
> > > > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <
> djacot@confluent.io
> > >
> > > > wrote:
> > > > > > > >
> > > > > > > > Hi Rajini,
> > > > > > > >
> > > > > > > > Thanks for the KIP. I have a few questions/comments:
> > > > > > > >
> > > > > > > > 01. If I understood correctly, the plan is to add new
> assignors
> > > > which
> > > > > > > > are rack aware. Is this right? I wonder if it is a judicious
> > > choice
> > > > > > > > here. The main drawback is that clients must be configured
> > > > correctly
> > > > > > > > in order to get the benefits. From a user experience
> > perspective,
> > > > it
> > > > > > > > would be much better if we would only require our users to
> set
> > > > > > > > client.rack. However, I understand the argument of keeping
> the
> > > > > > > > existing assignors as-is in order to limit the risk but it
> also
> > > > means
> > > > > > > > that we will have to maintain multiple assignors with a
> > somewhat
> > > > > > > > similar core logic (within a rack). What do you think?
> > > > > > > >
> > > > > > > > 02. If we proceed with new rack-aware assignors, we should
> > > mention
> > > > > > > > their fully qualified names in the KIP as they will become
> part
> > > of
> > > > our
> > > > > > > > public interfaces.
> > > > > > > >
> > > > > > > > 03. KIP-792 has already introduced version 2 of the
> > subscription.
> > > > The
> > > > > > > > KIP is accepted but the PR is not merged yet. This KIP should
> > use
> > > > > > > > version 3.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > David
> > > > > > > >
> > > > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > > > > >
> > > > > > > > > Hi Rajini,
> > > > > > > > >
> > > > > > > > > If I understand correctly, the client.rack config would
> stay
> > > > > > supported
> > > > > > > > > after KIP-848 but does it expand the scope of that KIP too
> > with
> > > > this
> > > > > > > > > config? I mean that currently you propose
> > > > > > ConsumerProtocolSubscription
> > > > > > > to
> > > > > > > > > be used but this protocol won't be available and we need to
> > > > transfer
> > > > > > > the
> > > > > > > > > config to the coordinator via other means. Should this be
> > added
> > > > to
> > > > > > > that KIP?
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Viktor
> > > > > > > > >
> > > > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > > > > rajinisivaram@gmail.com
> > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Jun,
> > > > > > > > > >
> > > > > > > > > > Thank you for the review. Yes, we should add rack id to
> > > > > > > Subscription, had
> > > > > > > > > > missed that part. Updated the KIP, thank you for pointing
> > > that
> > > > out!
> > > > > > > > > >
> > > > > > > > > > Regards,
> > > > > > > > > >
> > > > > > > > > > Rajini
> > > > > > > > > >
> > > > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> > > > <ju...@confluent.io.invalid>
> > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi, Rajini,
> > > > > > > > > > >
> > > > > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > > > > >
> > > > > > > > > > > Should we add rackId to GroupSubscription.Subscription
> > for
> > > > each
> > > > > > > member?
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > >
> > > > > > > > > > > Jun
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > > > > rajinisivaram@gmail.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi all,
> > > > > > > > > > > >
> > > > > > > > > > > > I have submitted KIP-881 to implement rack-aware
> > > partition
> > > > > > > assignment
> > > > > > > > > > for
> > > > > > > > > > > > consumers:
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > > > > .
> > > > > > > > > > > > It adds rack id to the consumer group protocol to
> > > propagate
> > > > > > rack
> > > > > > > > > > > > information so that rack-aware assignors can be added
> > to
> > > > > > benefit
> > > > > > > from
> > > > > > > > > > > > locality.
> > > > > > > > > > > >
> > > > > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > > > > >
> > > > > > > > > > > > Thank you,
> > > > > > > > > > > >
> > > > > > > > > > > > Rajini
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Artem Livshits <al...@confluent.io.INVALID>]

Hi Rajini,

Thank you for the KIP, the KIP looks good to match RackAwareReplicaSelector
behavior that is available out-of-box.  Which should probably be good
enough in practice.

From the design perspective, though, RackAwareReplicaSelector is just one
possible plugin, in theory the broker could use a plugin that leverages
networking information to get client locality or some other way, so it
seems like we're making an assumption about broker replica selection in the
default assignment implementation.  So I wonder if we should use a separate
plugin that would be set when RackAwareReplicaSelector is set, rather than
assume broker behavior in the client implementation.

-Artem

On Wed, Nov 16, 2022 at 8:08 AM Jun Rao <ju...@confluent.io.invalid> wrote:

> Hi, David and Rajini,
>
> Thanks for the explanation. It makes sense to me now.
>
> Jun
>
> On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Thanks David, that was my understanding as well.
> >
> > Regards,
> >
> > Rajini
> >
> > On Wed, Nov 16, 2022 at 8:08 AM David Jacot <djacot@confluent.io.invalid
> >
> > wrote:
> >
> > > Hi Jun,
> > >
> > > We don't need to bump any RPC requests. The subscription is serialized
> > > (including its version) and included as bytes in the RPCs.
> > >
> > > Best,
> > > David
> > >
> > > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao <ju...@confluent.io.invalid>
> > wrote:
> > > >
> > > > Hi, Rajini,
> > > >
> > > > Thanks for the updated KIP. Just another minor comment. It would be
> > > useful
> > > > to list all RPC requests whose version needs to be bumped because of
> > the
> > > > changes in ConsumerProtocolSubscription.
> > > >
> > > > Jun
> > > >
> > > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi David,
> > > > >
> > > > > Sorry, I was out of office and hence the delay in responding.
> Thanks
> > > for
> > > > > reviewing the KIP and answering Viktor's question (thanks for the
> > > review,
> > > > > Viktor).
> > > > >
> > > > > Responses below:
> > > > > 01)  I was in two minds about adding new assignors, because as you
> > > said,
> > > > > user experience is better if assignors used racks when available.
> But
> > > I was
> > > > > a bit concerned about changing the algorithm in existing
> applications
> > > which
> > > > > were already configuring `client.rack`. It felt less risky to add
> new
> > > > > assignor implementations instead. But we can retain existing logic
> if
> > > a)
> > > > > rack information is not available and b) racks have all partitions.
> > So
> > > the
> > > > > only case where logic will be different is when rack information is
> > > > > available because consumers chose to use `client.rack` to benefit
> > from
> > > > > improved locality, but racks only have a subset of partitions. It
> > seems
> > > > > reasonable to make existing assignors rack-aware in this case to
> > > improve
> > > > > locality. I have updated the KIP. Will wait and see if there are
> any
> > > > > objections to this change.
> > > > >
> > > > > 02) Updated 1), so existing assignor classes will be used.
> > > > >
> > > > > 03) Updated the KIP to use version 3, thanks.
> > > > >
> > > > > If there are no concerns or further comments, I will start voting
> > later
> > > > > today.
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Rajini
> > > > >
> > > > >
> > > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
> > <djacot@confluent.io.invalid
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Viktor,
> > > > > >
> > > > > > I can actually answer your question. KIP-848 already includes
> rack
> > > > > > awareness in the protocol. It is actually the other way around,
> > this
> > > > > > KIP takes the idea from KIP-848 to implement it in the current
> > > > > > protocol in order to realize the benefits sooner. The new
> protocol
> > > > > > will take a while to be implemented.
> > > > > >
> > > > > > Best,
> > > > > > David
> > > > > >
> > > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <djacot@confluent.io
> >
> > > wrote:
> > > > > > >
> > > > > > > Hi Rajini,
> > > > > > >
> > > > > > > Thanks for the KIP. I have a few questions/comments:
> > > > > > >
> > > > > > > 01. If I understood correctly, the plan is to add new assignors
> > > which
> > > > > > > are rack aware. Is this right? I wonder if it is a judicious
> > choice
> > > > > > > here. The main drawback is that clients must be configured
> > > correctly
> > > > > > > in order to get the benefits. From a user experience
> perspective,
> > > it
> > > > > > > would be much better if we would only require our users to set
> > > > > > > client.rack. However, I understand the argument of keeping the
> > > > > > > existing assignors as-is in order to limit the risk but it also
> > > means
> > > > > > > that we will have to maintain multiple assignors with a
> somewhat
> > > > > > > similar core logic (within a rack). What do you think?
> > > > > > >
> > > > > > > 02. If we proceed with new rack-aware assignors, we should
> > mention
> > > > > > > their fully qualified names in the KIP as they will become part
> > of
> > > our
> > > > > > > public interfaces.
> > > > > > >
> > > > > > > 03. KIP-792 has already introduced version 2 of the
> subscription.
> > > The
> > > > > > > KIP is accepted but the PR is not merged yet. This KIP should
> use
> > > > > > > version 3.
> > > > > > >
> > > > > > > Best,
> > > > > > > David
> > > > > > >
> > > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > > > >
> > > > > > > > Hi Rajini,
> > > > > > > >
> > > > > > > > If I understand correctly, the client.rack config would stay
> > > > > supported
> > > > > > > > after KIP-848 but does it expand the scope of that KIP too
> with
> > > this
> > > > > > > > config? I mean that currently you propose
> > > > > ConsumerProtocolSubscription
> > > > > > to
> > > > > > > > be used but this protocol won't be available and we need to
> > > transfer
> > > > > > the
> > > > > > > > config to the coordinator via other means. Should this be
> added
> > > to
> > > > > > that KIP?
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Viktor
> > > > > > > >
> > > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > > > rajinisivaram@gmail.com
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Jun,
> > > > > > > > >
> > > > > > > > > Thank you for the review. Yes, we should add rack id to
> > > > > > Subscription, had
> > > > > > > > > missed that part. Updated the KIP, thank you for pointing
> > that
> > > out!
> > > > > > > > >
> > > > > > > > > Regards,
> > > > > > > > >
> > > > > > > > > Rajini
> > > > > > > > >
> > > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> > > <ju...@confluent.io.invalid>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi, Rajini,
> > > > > > > > > >
> > > > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > > > >
> > > > > > > > > > Should we add rackId to GroupSubscription.Subscription
> for
> > > each
> > > > > > member?
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > >
> > > > > > > > > > Jun
> > > > > > > > > >
> > > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > > > rajinisivaram@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi all,
> > > > > > > > > > >
> > > > > > > > > > > I have submitted KIP-881 to implement rack-aware
> > partition
> > > > > > assignment
> > > > > > > > > for
> > > > > > > > > > > consumers:
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > > > .
> > > > > > > > > > > It adds rack id to the consumer group protocol to
> > propagate
> > > > > rack
> > > > > > > > > > > information so that rack-aware assignors can be added
> to
> > > > > benefit
> > > > > > from
> > > > > > > > > > > locality.
> > > > > > > > > > >
> > > > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > > > >
> > > > > > > > > > > Thank you,
> > > > > > > > > > >
> > > > > > > > > > > Rajini
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > >
> > > > >
> > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Jun Rao <ju...@confluent.io.INVALID>]

Hi, David and Rajini,

Thanks for the explanation. It makes sense to me now.

Jun

On Wed, Nov 16, 2022 at 1:44 AM Rajini Sivaram <ra...@gmail.com>
wrote:

> Thanks David, that was my understanding as well.
>
> Regards,
>
> Rajini
>
> On Wed, Nov 16, 2022 at 8:08 AM David Jacot <dj...@confluent.io.invalid>
> wrote:
>
> > Hi Jun,
> >
> > We don't need to bump any RPC requests. The subscription is serialized
> > (including its version) and included as bytes in the RPCs.
> >
> > Best,
> > David
> >
> > On Tue, Nov 15, 2022 at 11:42 PM Jun Rao <ju...@confluent.io.invalid>
> wrote:
> > >
> > > Hi, Rajini,
> > >
> > > Thanks for the updated KIP. Just another minor comment. It would be
> > useful
> > > to list all RPC requests whose version needs to be bumped because of
> the
> > > changes in ConsumerProtocolSubscription.
> > >
> > > Jun
> > >
> > > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > wrote:
> > >
> > > > Hi David,
> > > >
> > > > Sorry, I was out of office and hence the delay in responding. Thanks
> > for
> > > > reviewing the KIP and answering Viktor's question (thanks for the
> > review,
> > > > Viktor).
> > > >
> > > > Responses below:
> > > > 01)  I was in two minds about adding new assignors, because as you
> > said,
> > > > user experience is better if assignors used racks when available. But
> > I was
> > > > a bit concerned about changing the algorithm in existing applications
> > which
> > > > were already configuring `client.rack`. It felt less risky to add new
> > > > assignor implementations instead. But we can retain existing logic if
> > a)
> > > > rack information is not available and b) racks have all partitions.
> So
> > the
> > > > only case where logic will be different is when rack information is
> > > > available because consumers chose to use `client.rack` to benefit
> from
> > > > improved locality, but racks only have a subset of partitions. It
> seems
> > > > reasonable to make existing assignors rack-aware in this case to
> > improve
> > > > locality. I have updated the KIP. Will wait and see if there are any
> > > > objections to this change.
> > > >
> > > > 02) Updated 1), so existing assignor classes will be used.
> > > >
> > > > 03) Updated the KIP to use version 3, thanks.
> > > >
> > > > If there are no concerns or further comments, I will start voting
> later
> > > > today.
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > > >
> > > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot
> <djacot@confluent.io.invalid
> > >
> > > > wrote:
> > > >
> > > > > Hi Viktor,
> > > > >
> > > > > I can actually answer your question. KIP-848 already includes rack
> > > > > awareness in the protocol. It is actually the other way around,
> this
> > > > > KIP takes the idea from KIP-848 to implement it in the current
> > > > > protocol in order to realize the benefits sooner. The new protocol
> > > > > will take a while to be implemented.
> > > > >
> > > > > Best,
> > > > > David
> > > > >
> > > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <dj...@confluent.io>
> > wrote:
> > > > > >
> > > > > > Hi Rajini,
> > > > > >
> > > > > > Thanks for the KIP. I have a few questions/comments:
> > > > > >
> > > > > > 01. If I understood correctly, the plan is to add new assignors
> > which
> > > > > > are rack aware. Is this right? I wonder if it is a judicious
> choice
> > > > > > here. The main drawback is that clients must be configured
> > correctly
> > > > > > in order to get the benefits. From a user experience perspective,
> > it
> > > > > > would be much better if we would only require our users to set
> > > > > > client.rack. However, I understand the argument of keeping the
> > > > > > existing assignors as-is in order to limit the risk but it also
> > means
> > > > > > that we will have to maintain multiple assignors with a somewhat
> > > > > > similar core logic (within a rack). What do you think?
> > > > > >
> > > > > > 02. If we proceed with new rack-aware assignors, we should
> mention
> > > > > > their fully qualified names in the KIP as they will become part
> of
> > our
> > > > > > public interfaces.
> > > > > >
> > > > > > 03. KIP-792 has already introduced version 2 of the subscription.
> > The
> > > > > > KIP is accepted but the PR is not merged yet. This KIP should use
> > > > > > version 3.
> > > > > >
> > > > > > Best,
> > > > > > David
> > > > > >
> > > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > > >
> > > > > > > Hi Rajini,
> > > > > > >
> > > > > > > If I understand correctly, the client.rack config would stay
> > > > supported
> > > > > > > after KIP-848 but does it expand the scope of that KIP too with
> > this
> > > > > > > config? I mean that currently you propose
> > > > ConsumerProtocolSubscription
> > > > > to
> > > > > > > be used but this protocol won't be available and we need to
> > transfer
> > > > > the
> > > > > > > config to the coordinator via other means. Should this be added
> > to
> > > > > that KIP?
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Viktor
> > > > > > >
> > > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > > rajinisivaram@gmail.com
> > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Jun,
> > > > > > > >
> > > > > > > > Thank you for the review. Yes, we should add rack id to
> > > > > Subscription, had
> > > > > > > > missed that part. Updated the KIP, thank you for pointing
> that
> > out!
> > > > > > > >
> > > > > > > > Regards,
> > > > > > > >
> > > > > > > > Rajini
> > > > > > > >
> > > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> > <ju...@confluent.io.invalid>
> > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi, Rajini,
> > > > > > > > >
> > > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > > >
> > > > > > > > > Should we add rackId to GroupSubscription.Subscription for
> > each
> > > > > member?
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > >
> > > > > > > > > Jun
> > > > > > > > >
> > > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > > rajinisivaram@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi all,
> > > > > > > > > >
> > > > > > > > > > I have submitted KIP-881 to implement rack-aware
> partition
> > > > > assignment
> > > > > > > > for
> > > > > > > > > > consumers:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > > .
> > > > > > > > > > It adds rack id to the consumer group protocol to
> propagate
> > > > rack
> > > > > > > > > > information so that rack-aware assignors can be added to
> > > > benefit
> > > > > from
> > > > > > > > > > locality.
> > > > > > > > > >
> > > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > > >
> > > > > > > > > > Thank you,
> > > > > > > > > >
> > > > > > > > > > Rajini
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > >
> > > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Thanks David, that was my understanding as well.

Regards,

Rajini

On Wed, Nov 16, 2022 at 8:08 AM David Jacot <dj...@confluent.io.invalid>
wrote:

> Hi Jun,
>
> We don't need to bump any RPC requests. The subscription is serialized
> (including its version) and included as bytes in the RPCs.
>
> Best,
> David
>
> On Tue, Nov 15, 2022 at 11:42 PM Jun Rao <ju...@confluent.io.invalid> wrote:
> >
> > Hi, Rajini,
> >
> > Thanks for the updated KIP. Just another minor comment. It would be
> useful
> > to list all RPC requests whose version needs to be bumped because of the
> > changes in ConsumerProtocolSubscription.
> >
> > Jun
> >
> > On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi David,
> > >
> > > Sorry, I was out of office and hence the delay in responding. Thanks
> for
> > > reviewing the KIP and answering Viktor's question (thanks for the
> review,
> > > Viktor).
> > >
> > > Responses below:
> > > 01)  I was in two minds about adding new assignors, because as you
> said,
> > > user experience is better if assignors used racks when available. But
> I was
> > > a bit concerned about changing the algorithm in existing applications
> which
> > > were already configuring `client.rack`. It felt less risky to add new
> > > assignor implementations instead. But we can retain existing logic if
> a)
> > > rack information is not available and b) racks have all partitions. So
> the
> > > only case where logic will be different is when rack information is
> > > available because consumers chose to use `client.rack` to benefit from
> > > improved locality, but racks only have a subset of partitions. It seems
> > > reasonable to make existing assignors rack-aware in this case to
> improve
> > > locality. I have updated the KIP. Will wait and see if there are any
> > > objections to this change.
> > >
> > > 02) Updated 1), so existing assignor classes will be used.
> > >
> > > 03) Updated the KIP to use version 3, thanks.
> > >
> > > If there are no concerns or further comments, I will start voting later
> > > today.
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> > >
> > > On Fri, Nov 4, 2022 at 9:58 AM David Jacot <djacot@confluent.io.invalid
> >
> > > wrote:
> > >
> > > > Hi Viktor,
> > > >
> > > > I can actually answer your question. KIP-848 already includes rack
> > > > awareness in the protocol. It is actually the other way around, this
> > > > KIP takes the idea from KIP-848 to implement it in the current
> > > > protocol in order to realize the benefits sooner. The new protocol
> > > > will take a while to be implemented.
> > > >
> > > > Best,
> > > > David
> > > >
> > > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <dj...@confluent.io>
> wrote:
> > > > >
> > > > > Hi Rajini,
> > > > >
> > > > > Thanks for the KIP. I have a few questions/comments:
> > > > >
> > > > > 01. If I understood correctly, the plan is to add new assignors
> which
> > > > > are rack aware. Is this right? I wonder if it is a judicious choice
> > > > > here. The main drawback is that clients must be configured
> correctly
> > > > > in order to get the benefits. From a user experience perspective,
> it
> > > > > would be much better if we would only require our users to set
> > > > > client.rack. However, I understand the argument of keeping the
> > > > > existing assignors as-is in order to limit the risk but it also
> means
> > > > > that we will have to maintain multiple assignors with a somewhat
> > > > > similar core logic (within a rack). What do you think?
> > > > >
> > > > > 02. If we proceed with new rack-aware assignors, we should mention
> > > > > their fully qualified names in the KIP as they will become part of
> our
> > > > > public interfaces.
> > > > >
> > > > > 03. KIP-792 has already introduced version 2 of the subscription.
> The
> > > > > KIP is accepted but the PR is not merged yet. This KIP should use
> > > > > version 3.
> > > > >
> > > > > Best,
> > > > > David
> > > > >
> > > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > > <vi...@cloudera.com.invalid> wrote:
> > > > > >
> > > > > > Hi Rajini,
> > > > > >
> > > > > > If I understand correctly, the client.rack config would stay
> > > supported
> > > > > > after KIP-848 but does it expand the scope of that KIP too with
> this
> > > > > > config? I mean that currently you propose
> > > ConsumerProtocolSubscription
> > > > to
> > > > > > be used but this protocol won't be available and we need to
> transfer
> > > > the
> > > > > > config to the coordinator via other means. Should this be added
> to
> > > > that KIP?
> > > > > >
> > > > > > Thanks,
> > > > > > Viktor
> > > > > >
> > > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > > rajinisivaram@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Jun,
> > > > > > >
> > > > > > > Thank you for the review. Yes, we should add rack id to
> > > > Subscription, had
> > > > > > > missed that part. Updated the KIP, thank you for pointing that
> out!
> > > > > > >
> > > > > > > Regards,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao
> <ju...@confluent.io.invalid>
> > > > wrote:
> > > > > > >
> > > > > > > > Hi, Rajini,
> > > > > > > >
> > > > > > > > Thanks for the KIP. Just one comment.
> > > > > > > >
> > > > > > > > Should we add rackId to GroupSubscription.Subscription for
> each
> > > > member?
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > >
> > > > > > > > Jun
> > > > > > > >
> > > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > > rajinisivaram@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi all,
> > > > > > > > >
> > > > > > > > > I have submitted KIP-881 to implement rack-aware partition
> > > > assignment
> > > > > > > for
> > > > > > > > > consumers:
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > > .
> > > > > > > > > It adds rack id to the consumer group protocol to propagate
> > > rack
> > > > > > > > > information so that rack-aware assignors can be added to
> > > benefit
> > > > from
> > > > > > > > > locality.
> > > > > > > > >
> > > > > > > > > Feedback and suggestions are welcome!
> > > > > > > > >
> > > > > > > > > Thank you,
> > > > > > > > >
> > > > > > > > > Rajini
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > >
> > >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[David Jacot <dj...@confluent.io.INVALID>]

Hi Jun,

We don't need to bump any RPC requests. The subscription is serialized
(including its version) and included as bytes in the RPCs.

Best,
David

On Tue, Nov 15, 2022 at 11:42 PM Jun Rao <ju...@confluent.io.invalid> wrote:
>
> Hi, Rajini,
>
> Thanks for the updated KIP. Just another minor comment. It would be useful
> to list all RPC requests whose version needs to be bumped because of the
> changes in ConsumerProtocolSubscription.
>
> Jun
>
> On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi David,
> >
> > Sorry, I was out of office and hence the delay in responding. Thanks for
> > reviewing the KIP and answering Viktor's question (thanks for the review,
> > Viktor).
> >
> > Responses below:
> > 01)  I was in two minds about adding new assignors, because as you said,
> > user experience is better if assignors used racks when available. But I was
> > a bit concerned about changing the algorithm in existing applications which
> > were already configuring `client.rack`. It felt less risky to add new
> > assignor implementations instead. But we can retain existing logic if a)
> > rack information is not available and b) racks have all partitions. So the
> > only case where logic will be different is when rack information is
> > available because consumers chose to use `client.rack` to benefit from
> > improved locality, but racks only have a subset of partitions. It seems
> > reasonable to make existing assignors rack-aware in this case to improve
> > locality. I have updated the KIP. Will wait and see if there are any
> > objections to this change.
> >
> > 02) Updated 1), so existing assignor classes will be used.
> >
> > 03) Updated the KIP to use version 3, thanks.
> >
> > If there are no concerns or further comments, I will start voting later
> > today.
> >
> > Thank you,
> >
> > Rajini
> >
> >
> > On Fri, Nov 4, 2022 at 9:58 AM David Jacot <dj...@confluent.io.invalid>
> > wrote:
> >
> > > Hi Viktor,
> > >
> > > I can actually answer your question. KIP-848 already includes rack
> > > awareness in the protocol. It is actually the other way around, this
> > > KIP takes the idea from KIP-848 to implement it in the current
> > > protocol in order to realize the benefits sooner. The new protocol
> > > will take a while to be implemented.
> > >
> > > Best,
> > > David
> > >
> > > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <dj...@confluent.io> wrote:
> > > >
> > > > Hi Rajini,
> > > >
> > > > Thanks for the KIP. I have a few questions/comments:
> > > >
> > > > 01. If I understood correctly, the plan is to add new assignors which
> > > > are rack aware. Is this right? I wonder if it is a judicious choice
> > > > here. The main drawback is that clients must be configured correctly
> > > > in order to get the benefits. From a user experience perspective, it
> > > > would be much better if we would only require our users to set
> > > > client.rack. However, I understand the argument of keeping the
> > > > existing assignors as-is in order to limit the risk but it also means
> > > > that we will have to maintain multiple assignors with a somewhat
> > > > similar core logic (within a rack). What do you think?
> > > >
> > > > 02. If we proceed with new rack-aware assignors, we should mention
> > > > their fully qualified names in the KIP as they will become part of our
> > > > public interfaces.
> > > >
> > > > 03. KIP-792 has already introduced version 2 of the subscription. The
> > > > KIP is accepted but the PR is not merged yet. This KIP should use
> > > > version 3.
> > > >
> > > > Best,
> > > > David
> > > >
> > > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > > <vi...@cloudera.com.invalid> wrote:
> > > > >
> > > > > Hi Rajini,
> > > > >
> > > > > If I understand correctly, the client.rack config would stay
> > supported
> > > > > after KIP-848 but does it expand the scope of that KIP too with this
> > > > > config? I mean that currently you propose
> > ConsumerProtocolSubscription
> > > to
> > > > > be used but this protocol won't be available and we need to transfer
> > > the
> > > > > config to the coordinator via other means. Should this be added to
> > > that KIP?
> > > > >
> > > > > Thanks,
> > > > > Viktor
> > > > >
> > > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> > rajinisivaram@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Jun,
> > > > > >
> > > > > > Thank you for the review. Yes, we should add rack id to
> > > Subscription, had
> > > > > > missed that part. Updated the KIP, thank you for pointing that out!
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao <ju...@confluent.io.invalid>
> > > wrote:
> > > > > >
> > > > > > > Hi, Rajini,
> > > > > > >
> > > > > > > Thanks for the KIP. Just one comment.
> > > > > > >
> > > > > > > Should we add rackId to GroupSubscription.Subscription for each
> > > member?
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Jun
> > > > > > >
> > > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > I have submitted KIP-881 to implement rack-aware partition
> > > assignment
> > > > > > for
> > > > > > > > consumers:
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > > .
> > > > > > > > It adds rack id to the consumer group protocol to propagate
> > rack
> > > > > > > > information so that rack-aware assignors can be added to
> > benefit
> > > from
> > > > > > > > locality.
> > > > > > > >
> > > > > > > > Feedback and suggestions are welcome!
> > > > > > > >
> > > > > > > > Thank you,
> > > > > > > >
> > > > > > > > Rajini
> > > > > > > >
> > > > > > >
> > > > > >
> > >
> >

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Jun Rao <ju...@confluent.io.INVALID>]

Hi, Rajini,

Thanks for the updated KIP. Just another minor comment. It would be useful
to list all RPC requests whose version needs to be bumped because of the
changes in ConsumerProtocolSubscription.

Jun

On Tue, Nov 15, 2022 at 3:45 AM Rajini Sivaram <ra...@gmail.com>
wrote:

> Hi David,
>
> Sorry, I was out of office and hence the delay in responding. Thanks for
> reviewing the KIP and answering Viktor's question (thanks for the review,
> Viktor).
>
> Responses below:
> 01)  I was in two minds about adding new assignors, because as you said,
> user experience is better if assignors used racks when available. But I was
> a bit concerned about changing the algorithm in existing applications which
> were already configuring `client.rack`. It felt less risky to add new
> assignor implementations instead. But we can retain existing logic if a)
> rack information is not available and b) racks have all partitions. So the
> only case where logic will be different is when rack information is
> available because consumers chose to use `client.rack` to benefit from
> improved locality, but racks only have a subset of partitions. It seems
> reasonable to make existing assignors rack-aware in this case to improve
> locality. I have updated the KIP. Will wait and see if there are any
> objections to this change.
>
> 02) Updated 1), so existing assignor classes will be used.
>
> 03) Updated the KIP to use version 3, thanks.
>
> If there are no concerns or further comments, I will start voting later
> today.
>
> Thank you,
>
> Rajini
>
>
> On Fri, Nov 4, 2022 at 9:58 AM David Jacot <dj...@confluent.io.invalid>
> wrote:
>
> > Hi Viktor,
> >
> > I can actually answer your question. KIP-848 already includes rack
> > awareness in the protocol. It is actually the other way around, this
> > KIP takes the idea from KIP-848 to implement it in the current
> > protocol in order to realize the benefits sooner. The new protocol
> > will take a while to be implemented.
> >
> > Best,
> > David
> >
> > On Fri, Nov 4, 2022 at 10:55 AM David Jacot <dj...@confluent.io> wrote:
> > >
> > > Hi Rajini,
> > >
> > > Thanks for the KIP. I have a few questions/comments:
> > >
> > > 01. If I understood correctly, the plan is to add new assignors which
> > > are rack aware. Is this right? I wonder if it is a judicious choice
> > > here. The main drawback is that clients must be configured correctly
> > > in order to get the benefits. From a user experience perspective, it
> > > would be much better if we would only require our users to set
> > > client.rack. However, I understand the argument of keeping the
> > > existing assignors as-is in order to limit the risk but it also means
> > > that we will have to maintain multiple assignors with a somewhat
> > > similar core logic (within a rack). What do you think?
> > >
> > > 02. If we proceed with new rack-aware assignors, we should mention
> > > their fully qualified names in the KIP as they will become part of our
> > > public interfaces.
> > >
> > > 03. KIP-792 has already introduced version 2 of the subscription. The
> > > KIP is accepted but the PR is not merged yet. This KIP should use
> > > version 3.
> > >
> > > Best,
> > > David
> > >
> > > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > > <vi...@cloudera.com.invalid> wrote:
> > > >
> > > > Hi Rajini,
> > > >
> > > > If I understand correctly, the client.rack config would stay
> supported
> > > > after KIP-848 but does it expand the scope of that KIP too with this
> > > > config? I mean that currently you propose
> ConsumerProtocolSubscription
> > to
> > > > be used but this protocol won't be available and we need to transfer
> > the
> > > > config to the coordinator via other means. Should this be added to
> > that KIP?
> > > >
> > > > Thanks,
> > > > Viktor
> > > >
> > > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <
> rajinisivaram@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Hi Jun,
> > > > >
> > > > > Thank you for the review. Yes, we should add rack id to
> > Subscription, had
> > > > > missed that part. Updated the KIP, thank you for pointing that out!
> > > > >
> > > > > Regards,
> > > > >
> > > > > Rajini
> > > > >
> > > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao <ju...@confluent.io.invalid>
> > wrote:
> > > > >
> > > > > > Hi, Rajini,
> > > > > >
> > > > > > Thanks for the KIP. Just one comment.
> > > > > >
> > > > > > Should we add rackId to GroupSubscription.Subscription for each
> > member?
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> > rajinisivaram@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I have submitted KIP-881 to implement rack-aware partition
> > assignment
> > > > > for
> > > > > > > consumers:
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > > .
> > > > > > > It adds rack id to the consumer group protocol to propagate
> rack
> > > > > > > information so that rack-aware assignors can be added to
> benefit
> > from
> > > > > > > locality.
> > > > > > >
> > > > > > > Feedback and suggestions are welcome!
> > > > > > >
> > > > > > > Thank you,
> > > > > > >
> > > > > > > Rajini
> > > > > > >
> > > > > >
> > > > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Hi David,

Sorry, I was out of office and hence the delay in responding. Thanks for
reviewing the KIP and answering Viktor's question (thanks for the review,
Viktor).

Responses below:
01)  I was in two minds about adding new assignors, because as you said,
user experience is better if assignors used racks when available. But I was
a bit concerned about changing the algorithm in existing applications which
were already configuring `client.rack`. It felt less risky to add new
assignor implementations instead. But we can retain existing logic if a)
rack information is not available and b) racks have all partitions. So the
only case where logic will be different is when rack information is
available because consumers chose to use `client.rack` to benefit from
improved locality, but racks only have a subset of partitions. It seems
reasonable to make existing assignors rack-aware in this case to improve
locality. I have updated the KIP. Will wait and see if there are any
objections to this change.

02) Updated 1), so existing assignor classes will be used.

03) Updated the KIP to use version 3, thanks.

If there are no concerns or further comments, I will start voting later
today.

Thank you,

Rajini


On Fri, Nov 4, 2022 at 9:58 AM David Jacot <dj...@confluent.io.invalid>
wrote:

> Hi Viktor,
>
> I can actually answer your question. KIP-848 already includes rack
> awareness in the protocol. It is actually the other way around, this
> KIP takes the idea from KIP-848 to implement it in the current
> protocol in order to realize the benefits sooner. The new protocol
> will take a while to be implemented.
>
> Best,
> David
>
> On Fri, Nov 4, 2022 at 10:55 AM David Jacot <dj...@confluent.io> wrote:
> >
> > Hi Rajini,
> >
> > Thanks for the KIP. I have a few questions/comments:
> >
> > 01. If I understood correctly, the plan is to add new assignors which
> > are rack aware. Is this right? I wonder if it is a judicious choice
> > here. The main drawback is that clients must be configured correctly
> > in order to get the benefits. From a user experience perspective, it
> > would be much better if we would only require our users to set
> > client.rack. However, I understand the argument of keeping the
> > existing assignors as-is in order to limit the risk but it also means
> > that we will have to maintain multiple assignors with a somewhat
> > similar core logic (within a rack). What do you think?
> >
> > 02. If we proceed with new rack-aware assignors, we should mention
> > their fully qualified names in the KIP as they will become part of our
> > public interfaces.
> >
> > 03. KIP-792 has already introduced version 2 of the subscription. The
> > KIP is accepted but the PR is not merged yet. This KIP should use
> > version 3.
> >
> > Best,
> > David
> >
> > On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> > <vi...@cloudera.com.invalid> wrote:
> > >
> > > Hi Rajini,
> > >
> > > If I understand correctly, the client.rack config would stay supported
> > > after KIP-848 but does it expand the scope of that KIP too with this
> > > config? I mean that currently you propose ConsumerProtocolSubscription
> to
> > > be used but this protocol won't be available and we need to transfer
> the
> > > config to the coordinator via other means. Should this be added to
> that KIP?
> > >
> > > Thanks,
> > > Viktor
> > >
> > > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <rajinisivaram@gmail.com
> >
> > > wrote:
> > >
> > > > Hi Jun,
> > > >
> > > > Thank you for the review. Yes, we should add rack id to
> Subscription, had
> > > > missed that part. Updated the KIP, thank you for pointing that out!
> > > >
> > > > Regards,
> > > >
> > > > Rajini
> > > >
> > > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao <ju...@confluent.io.invalid>
> wrote:
> > > >
> > > > > Hi, Rajini,
> > > > >
> > > > > Thanks for the KIP. Just one comment.
> > > > >
> > > > > Should we add rackId to GroupSubscription.Subscription for each
> member?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <
> rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > I have submitted KIP-881 to implement rack-aware partition
> assignment
> > > > for
> > > > > > consumers:
> > > > > >
> > > > > >
> > > > >
> > > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > > .
> > > > > > It adds rack id to the consumer group protocol to propagate rack
> > > > > > information so that rack-aware assignors can be added to benefit
> from
> > > > > > locality.
> > > > > >
> > > > > > Feedback and suggestions are welcome!
> > > > > >
> > > > > > Thank you,
> > > > > >
> > > > > > Rajini
> > > > > >
> > > > >
> > > >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[David Jacot <dj...@confluent.io.INVALID>]

Hi Viktor,

I can actually answer your question. KIP-848 already includes rack
awareness in the protocol. It is actually the other way around, this
KIP takes the idea from KIP-848 to implement it in the current
protocol in order to realize the benefits sooner. The new protocol
will take a while to be implemented.

Best,
David

On Fri, Nov 4, 2022 at 10:55 AM David Jacot <dj...@confluent.io> wrote:
>
> Hi Rajini,
>
> Thanks for the KIP. I have a few questions/comments:
>
> 01. If I understood correctly, the plan is to add new assignors which
> are rack aware. Is this right? I wonder if it is a judicious choice
> here. The main drawback is that clients must be configured correctly
> in order to get the benefits. From a user experience perspective, it
> would be much better if we would only require our users to set
> client.rack. However, I understand the argument of keeping the
> existing assignors as-is in order to limit the risk but it also means
> that we will have to maintain multiple assignors with a somewhat
> similar core logic (within a rack). What do you think?
>
> 02. If we proceed with new rack-aware assignors, we should mention
> their fully qualified names in the KIP as they will become part of our
> public interfaces.
>
> 03. KIP-792 has already introduced version 2 of the subscription. The
> KIP is accepted but the PR is not merged yet. This KIP should use
> version 3.
>
> Best,
> David
>
> On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
> <vi...@cloudera.com.invalid> wrote:
> >
> > Hi Rajini,
> >
> > If I understand correctly, the client.rack config would stay supported
> > after KIP-848 but does it expand the scope of that KIP too with this
> > config? I mean that currently you propose ConsumerProtocolSubscription to
> > be used but this protocol won't be available and we need to transfer the
> > config to the coordinator via other means. Should this be added to that KIP?
> >
> > Thanks,
> > Viktor
> >
> > On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi Jun,
> > >
> > > Thank you for the review. Yes, we should add rack id to Subscription, had
> > > missed that part. Updated the KIP, thank you for pointing that out!
> > >
> > > Regards,
> > >
> > > Rajini
> > >
> > > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao <ju...@confluent.io.invalid> wrote:
> > >
> > > > Hi, Rajini,
> > > >
> > > > Thanks for the KIP. Just one comment.
> > > >
> > > > Should we add rackId to GroupSubscription.Subscription for each member?
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <ra...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi all,
> > > > >
> > > > > I have submitted KIP-881 to implement rack-aware partition assignment
> > > for
> > > > > consumers:
> > > > >
> > > > >
> > > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > > .
> > > > > It adds rack id to the consumer group protocol to propagate rack
> > > > > information so that rack-aware assignors can be added to benefit from
> > > > > locality.
> > > > >
> > > > > Feedback and suggestions are welcome!
> > > > >
> > > > > Thank you,
> > > > >
> > > > > Rajini
> > > > >
> > > >
> > >

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[David Jacot <dj...@confluent.io.INVALID>]

Hi Rajini,

Thanks for the KIP. I have a few questions/comments:

01. If I understood correctly, the plan is to add new assignors which
are rack aware. Is this right? I wonder if it is a judicious choice
here. The main drawback is that clients must be configured correctly
in order to get the benefits. From a user experience perspective, it
would be much better if we would only require our users to set
client.rack. However, I understand the argument of keeping the
existing assignors as-is in order to limit the risk but it also means
that we will have to maintain multiple assignors with a somewhat
similar core logic (within a rack). What do you think?

02. If we proceed with new rack-aware assignors, we should mention
their fully qualified names in the KIP as they will become part of our
public interfaces.

03. KIP-792 has already introduced version 2 of the subscription. The
KIP is accepted but the PR is not merged yet. This KIP should use
version 3.

Best,
David

On Thu, Nov 3, 2022 at 5:58 PM Viktor Somogyi-Vass
<vi...@cloudera.com.invalid> wrote:
>
> Hi Rajini,
>
> If I understand correctly, the client.rack config would stay supported
> after KIP-848 but does it expand the scope of that KIP too with this
> config? I mean that currently you propose ConsumerProtocolSubscription to
> be used but this protocol won't be available and we need to transfer the
> config to the coordinator via other means. Should this be added to that KIP?
>
> Thanks,
> Viktor
>
> On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi Jun,
> >
> > Thank you for the review. Yes, we should add rack id to Subscription, had
> > missed that part. Updated the KIP, thank you for pointing that out!
> >
> > Regards,
> >
> > Rajini
> >
> > On Wed, Nov 2, 2022 at 7:06 PM Jun Rao <ju...@confluent.io.invalid> wrote:
> >
> > > Hi, Rajini,
> > >
> > > Thanks for the KIP. Just one comment.
> > >
> > > Should we add rackId to GroupSubscription.Subscription for each member?
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <ra...@gmail.com>
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > I have submitted KIP-881 to implement rack-aware partition assignment
> > for
> > > > consumers:
> > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > > .
> > > > It adds rack id to the consumer group protocol to propagate rack
> > > > information so that rack-aware assignors can be added to benefit from
> > > > locality.
> > > >
> > > > Feedback and suggestions are welcome!
> > > >
> > > > Thank you,
> > > >
> > > > Rajini
> > > >
> > >
> >

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Viktor Somogyi-Vass <vi...@cloudera.com.INVALID>]

Hi Rajini,

If I understand correctly, the client.rack config would stay supported
after KIP-848 but does it expand the scope of that KIP too with this
config? I mean that currently you propose ConsumerProtocolSubscription to
be used but this protocol won't be available and we need to transfer the
config to the coordinator via other means. Should this be added to that KIP?

Thanks,
Viktor

On Wed, Nov 2, 2022 at 9:50 PM Rajini Sivaram <ra...@gmail.com>
wrote:

> Hi Jun,
>
> Thank you for the review. Yes, we should add rack id to Subscription, had
> missed that part. Updated the KIP, thank you for pointing that out!
>
> Regards,
>
> Rajini
>
> On Wed, Nov 2, 2022 at 7:06 PM Jun Rao <ju...@confluent.io.invalid> wrote:
>
> > Hi, Rajini,
> >
> > Thanks for the KIP. Just one comment.
> >
> > Should we add rackId to GroupSubscription.Subscription for each member?
> >
> > Thanks,
> >
> > Jun
> >
> > On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <ra...@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > I have submitted KIP-881 to implement rack-aware partition assignment
> for
> > > consumers:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > > .
> > > It adds rack id to the consumer group protocol to propagate rack
> > > information so that rack-aware assignors can be added to benefit from
> > > locality.
> > >
> > > Feedback and suggestions are welcome!
> > >
> > > Thank you,
> > >
> > > Rajini
> > >
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Rajini Sivaram <ra...@gmail.com>]

Hi Jun,

Thank you for the review. Yes, we should add rack id to Subscription, had
missed that part. Updated the KIP, thank you for pointing that out!

Regards,

Rajini

On Wed, Nov 2, 2022 at 7:06 PM Jun Rao <ju...@confluent.io.invalid> wrote:

> Hi, Rajini,
>
> Thanks for the KIP. Just one comment.
>
> Should we add rackId to GroupSubscription.Subscription for each member?
>
> Thanks,
>
> Jun
>
> On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <ra...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I have submitted KIP-881 to implement rack-aware partition assignment for
> > consumers:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> > .
> > It adds rack id to the consumer group protocol to propagate rack
> > information so that rack-aware assignors can be added to benefit from
> > locality.
> >
> > Feedback and suggestions are welcome!
> >
> > Thank you,
> >
> > Rajini
> >
>

Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers

[Jun Rao <ju...@confluent.io.INVALID>]

Hi, Rajini,

Thanks for the KIP. Just one comment.

Should we add rackId to GroupSubscription.Subscription for each member?

Thanks,

Jun

On Wed, Nov 2, 2022 at 4:57 AM Rajini Sivaram <ra...@gmail.com>
wrote:

> Hi all,
>
> I have submitted KIP-881 to implement rack-aware partition assignment for
> consumers:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers
> .
> It adds rack id to the consumer group protocol to propagate rack
> information so that rack-aware assignors can be added to benefit from
> locality.
>
> Feedback and suggestions are welcome!
>
> Thank you,
>
> Rajini
>