You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Rajini Sivaram <ra...@gmail.com> on 2022/12/01 09:12:31 UTC
Re: [DISCUSS] KIP-881: Rack-aware Partition Assignment for Kafka Consumers
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
Posted by 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
>> > > > > > > > > > > > > > > >
>> > > > > > > > > > > > > > >
>> > > > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>