Hi Tina,
Great stuff, LGTM.
Cheers,
Chris
On Mon, Feb 20, 2023 at 8:12 AM Gantigmaa Selenge
wrote:
> Hi Chris,
>
> I have fixed the nits you mentioned and tried to improve the flow a little
> bit.
>
> Please let me know what you think :) .
>
> Thank you.
> Regards,
> Tina
>
> On Fri, Feb 17,
Hi Chris,
I have fixed the nits you mentioned and tried to improve the flow a little
bit.
Please let me know what you think :) .
Thank you.
Regards,
Tina
On Fri, Feb 17, 2023 at 6:15 PM Chris Egerton
wrote:
> Hi Tina,
>
> This is looking great. I have a few nits remaining but apart from those
Hi Tina,
This is looking great. I have a few nits remaining but apart from those I'm
happy with the KIP and ready to vote.
1. In the description for how MM2 will behave when configured with
"use.incremental.alter.configs" set to "requested", the KIP states that "If
the first request receives an e
Hi Chris,
> - The incremental API is used
- ConfigPropertyFilter::shouldReplicateConfigProperty returns true
- ConfigPropertyFilter::shouldReplicateSourceDefault returns false
This sounds good to me. So just to clarify this in my head, when
incremental API is used, the MM2 will check shouldReplic
Hi Tina,
It's looking better! A few thoughts:
I think we should clarify in the KIP that under these conditions, MM2 will
explicitly wipe properties from topic configs on the target cluster (via a
DELETE operation):
- The incremental API is used
- ConfigPropertyFilter::shouldReplicateConfigPropert
Thank you Chris. I agree with what you have suggested.
I have updated the KIP, please let me know if I missed anything or if there
is any other question.
Regards,
Tina
On Tue, Feb 14, 2023 at 4:40 PM Chris Egerton
wrote:
> Hi Tina,
>
> While I agree that it's reasonable for users to want to fav
Hi Tina,
While I agree that it's reasonable for users to want to favor the source
cluster's defaults over the target cluster's, I'm hesitant to change this
behavior in an opt-out fashion. IMO it's better to allow users to opt into
this (by adding a method to the ConfigPropertyFilter interface, and
Hi Chris
My comment on the second point is not correct. Please ignore the part about
the config source (config source does set back to DEFAULT_CONFIG when
deleting a config). I got diverted off the issue a little bit.
With the legacy API, we do propagate deletion due to resetting all the
configs
Hi Tina,
Thanks for the updates!
RE item 1: Ah, to be clear, I wasn't suggesting that we retry with the
incremental API for every request, just wondering about whether we'd want
to block on the very first request completing before we issue subsequent
requests, since otherwise we might send multip
Hi Chris and Luke,
Thank you very much for your feedback.
I have addressed some of the suggestions and would like to clarify a few
things on the others:
> 1) The current logic for syncing topic configs in MM2 is basically
fire-and-forget; all we do is log a warning message [1] if an attempt
fail
Hi Tina,
Thanks for the KIP to fix the issue.
Some comments:
1. In the compatibility section, you said:
`By default, the new setting will be set to false so it does not change the
current behaviour.`
I'm confused, what is the config we need to set to `false` to avoid
breaking compatibility?
All
Hi Tina,
Thanks for the KIP! I recently ran into this exact issue and it's great to
see a fix being proposed. I have a few small comments but overall this
looks good:
1) The current logic for syncing topic configs in MM2 is basically
fire-and-forget; all we do is log a warning message [1] if an a
Hi,
If there are no further comments on the KIP, I will start a vote on it.
Regards,
On Mon, Jan 16, 2023 at 11:14 AM Gantigmaa Selenge
wrote:
> Thanks everyone.
>
> I took the suggestions and updated the KIP accordingly. Please let me know
> if there is anything else I could improve on.
>
>
Thanks everyone.
I took the suggestions and updated the KIP accordingly. Please let me know
if there is anything else I could improve on.
Regards,
Tina
On Sun, Jan 15, 2023 at 10:24 PM Ismael Juma wrote:
> Hi Tina,
>
> See below.
>
> On Wed, Jan 11, 2023 at 3:03 AM Gantigmaa Selenge
> wrote:
Hi Tina,
See below.
On Wed, Jan 11, 2023 at 3:03 AM Gantigmaa Selenge
wrote:
> 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
On Wed, Jan 11, 2023 at 12:03 PM Gantigmaa Selenge 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 a
On Sat, Jan 7, 2023, at 11:39, Ismael Juma 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 b
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,
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
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
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-89
21 matches
Mail list logo
