Hi all,

Can we clarify any changes in the user-facing semantics for the CLI tool
that would come about as a result of this KIP? I think the debate over the
necessity of an opt-in flag, or waiting for 4.0.0, ultimately boils down to
this.

My understanding is that the only changes in semantics are fairly minor
(semantic versioning pun intended):

- Existing sensitive broker properties no longer have to be explicitly
specified on the command line if they're not being changed
- A small race condition is fixed where the broker config is updated by a
separate operation in between when the CLI reads the existing broker config
and writes the new broker config
- Usage of a new broker API that has been supported since version 2.3.0,
but which does not require any new ACLs and does not act any differently
apart from the two small changes noted above

If this is correct, then I'm inclined to agree with Ismael's suggestion of
starting with incrementalAlterConfigs, and falling back on alterConfigs if
the former is not supported by the broker, and do not believe it's
necessary to wait for 4.0.0 or provide opt-in or opt-out flags to release
this change. This would also be similar to changes we made to MirrorMaker 2
in KIP-894 [1], where the default behavior for syncing topic configs is now
to start with incrementalAlterConfigs and fall back on alterConfigs if it's
not supported.

If there are other, more significant changes to the user-facing semantics
for the CLI, then these should be called out here and in the KIP, and we
might consider a more cautious approach.

[1] -
https://cwiki.apache.org/confluence/display/KAFKA/KIP-894%3A+Use+incrementalAlterConfigs+API+for+syncing+topic+configurations


Also, regarding this part of the KIP:

> incrementalAlterConfigs is more convenient especially for updating
configs of list data type, such as "leader.replication.throttled.replicas"

While this is true for the Java admin client and the corresponding broker
APIs, it doesn't appear to be relevant to the kafka-configs.sh CLI tool. We
don't appear to support appending/subtracting from list properties via the
CLI for any other entity type right now, and there's nothing in the KIP
that leads me to believe we'd be adding it for broker configs.

Cheers,

Chris

On Thu, Jan 4, 2024 at 10:12 PM ziming deng <dengziming1...@gmail.com>
wrote:

> Hi Ismael,
> I added this automatically approach to “Rejected alternatives” concerning
> that we need to unify the semantics between alterConfigs and
> incrementalAlterConfigs, so I choose to give this privilege to users.
>
> After reviewing these code and doing some tests I found that they
> following the similar approach, I think the simplest way is to let the
> client choose the best method heuristically.
>
> Thank you for pointing out this, I will change the KIP later.
>
> Best,
> Ziming
>
> > On Jan 4, 2024, at 17:28, Ismael Juma <m...@ismaeljuma.com> wrote:
> >
> > Hi Ziming,
> >
> > Why is the flag required at all? Can we use incremental and fallback
> automatically if it's not supported by the broker? At this point, the vast
> majority of clusters should support it.
> >
> > Ismael
> >
> > On Mon, Dec 18, 2023 at 7:58 PM ziming deng <dengziming1...@gmail.com
> <mailto:dengziming1...@gmail.com>> wrote:
> >>
> >> Hello, I want to start a discussion on KIP-1011, to make the broker
> config change path unified with that of user/topic/client-metrics and avoid
> some bugs.
> >>
> >> Here is the link:
> >>
> >> KIP-1011: Use incrementalAlterConfigs when updating broker configs by
> kafka-configs.sh - Apache Kafka - Apache Software Foundation
> >> cwiki.apache.org
> >>
> >>  <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1011%3A+Use+incrementalAlterConfigs+when+updating+broker+configs+by+kafka-configs.sh>KIP-1011:
> Use incrementalAlterConfigs when updating broker configs by
> kafka-configs.sh - Apache Kafka - Apache Software Foundation <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1011%3A+Use+incrementalAlterConfigs+when+updating+broker+configs+by+kafka-configs.sh
> >
> >> cwiki.apache.org <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1011%3A+Use+incrementalAlterConfigs+when+updating+broker+configs+by+kafka-configs.sh>
>        <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1011%3A+Use+incrementalAlterConfigs+when+updating+broker+configs+by+kafka-configs.sh
> >
> >>
> >> Best,
> >> Ziming.
>
>

Reply via email to