On Wed, Jan 11, 2023 at 12:03 PM Gantigmaa Selenge <gsele...@redhat.com> wrote: > > Thank you both for the feedback. > > > Have we considered using incremental alter configs by > default and fallback to the legacy one if the former is unavailable? > Initially having a flag with just on and off switches seemed simpler and it > gives users control and awareness of what's being used. However, now I > think using incremental alter configs by default and if it is unavailable, > fallback to the legacy API is a good idea. > > > The config could have 3 possible values: requested, required, never. The > default would be requested. > > I do like this idea, however when it's set to required, I wasn't sure how > the mirrormaker should have. It's probably not a great experience if > mirrormaker starts crashing at some point after it's already running due to > an incompatible broker version. > > If the incrementalAlterConfig request fails because the target cluster is > running an older version, then we could log a WARN message that says > something like "The config to use incrementalAlterConfig API for syncing > topic configurations has been set to true however target cluster is running > incompatible version therefore using the legacy alterConfig API". This way > the Mirrormaker never has to stop working and makes the user aware of > what's being used. In this case, we also would not need 3 separate values > for the config, instead would use the original true or false values: > - true - > would use incrementalAlterConfig API, but if it's unavailable, > fallback to the legacy API > - false -> keep using the legacy API > > Set the flag to true by default and remove the config in 4.0.
Hi Tina, I'm in favor of this variant. Maybe the warning message could just be "The target cluster <ALIAS> is not compatible with the IncrementalAlterConfigs API, falling back to AlterConfigs API". Thanks. > > > One suggestion: I'm not sure how concerned you are about people's ability > to migrate, but if you want to make it as smooth as possible, you could add > one more step. In the 4.0 release, while removing > `use.incremental.alter.configs`, you can also add > `use.legacy.alter.configs` to give users a path to continue using the old > behavior even after the default changes. > > If we implement the fallback logic as Ismael suggested above, I think we > would not need this extra flag later anymore. > > Please let me know what you think. Then I can go ahead and update the KIP. > > Regards, > Tina > > On Sat, Jan 7, 2023 at 7:45 PM Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi, > > > > Thanks for the KIP. Have we considered using incremental alter configs by > > default and fallback to the legacy one if the former is unavailable? > > > > The config could have 3 possible values: requested, required, never. The > > default would be requested. In 4.0, this could would be removed and it > > would effectively be required. > > > > Ismael > > > > On Sat, Jan 7, 2023, 10:03 AM John Roesler <vvcep...@apache.org> wrote: > > > > > Hi Tina, > > > > > > Thanks for the KIP! > > > > > > I hope someone with prior MM or Kafka config experience is able to chime > > > in here; I have neither. > > > > > > I took a look at your KIP, and it makes sense to me. I also think your > > > migration plan is a good one. > > > > > > One suggestion: I'm not sure how concerned you are about people's ability > > > to migrate, but if you want to make it as smooth as possible, you could > > add > > > one more step. In the 4.0 release, while removing > > > `use.incremental.alter.configs`, you can also add > > > `use.legacy.alter.configs` to give users a path to continue using the old > > > behavior even after the default changes. > > > > > > Clearly, this will prolong the deprecation period, with implications on > > > code maintenance, so there is some downside. But generally, I've found > > > going above and beyond to support smooth upgrades for users to be well > > > worth it in the long run. > > > > > > Thanks again, > > > -John > > > > > > > > > On Fri, Jan 6, 2023, at 05:49, Gantigmaa Selenge wrote: > > > > Hi everyone, > > > > > > > > I would like to start a discussion on the MirrorMaker update that > > > > proposes > > > > replacing the deprecated alterConfigs API with the > > > > incrementalAlterConfigs > > > > API for syncing topic configurations. Please take a look at the > > proposal > > > > here: > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-894%3A+Use+incrementalAlterConfigs+API+for+syncing+topic+configurations > > > > > > > > > > > > Regards, > > > > Tina > > > > >
