Thank you for your clarification, Chris, I have spent some time to review KIP-894 and I think it's automatic way is better and bring no side effect, and I will also adopt this way here. As you mentioned, the changes in semantics is minor, the most important reason for this change is fixing bug brought by sensitive configs.
> We > don't appear to support appending/subtracting from list properties via the > CLI for any other entity type right now, You are right about this, I tried and found that we can’t subtract or append configs, I will change the KIP to "making way for appending/subtracting list properties" -- Best, Ziming > On Jan 6, 2024, at 01:34, Chris Egerton <chr...@aiven.io.INVALID> wrote: > > 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 > <mailto: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 <http://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 <http://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.
