Thanks for both of your feedback and time, I updated the rejected
alternative section in the KIP and opened a voting thread here
https://lists.apache.org/thread/cy04n445noyp0pqztlp8rk74crvhlrk7
I'll work on the PR in meanwhile so we are ready to go once we get 3
binding votes in order to get into
Hi, the point that the legacy approach can only be taken once is
valid, so LGTM. Thanks.
Cheers
Fede
On Fri, Jul 21, 2023 at 4:28 PM Chris Egerton wrote:
>
> Hi Omnia,
>
> LGTM, thanks! We may want to note the LegacyReplicationPolicy option in the
> rejected alternatives section in case others
Hi Omnia,
LGTM, thanks! We may want to note the LegacyReplicationPolicy option in the
rejected alternatives section in case others prefer that option.
Given that we'd like this to land in time for 3.6.0, you may want to start
a vote thread soon.
Cheers,
Chris
On Fri, Jul 21, 2023 at 10:08 AM
Hi Chris and Federico,
thinking about I think Chris's concern is valid and the bigger concern here
is that having this `LegacyReplicationPolicy` will eventually open the door
for diversion at some point between this `LegacyReplicationPolicy` and the
default one.
For now, let's have the flag
Hi Federico,
Ah yes, sorry about that! You're correct that this would keep the two
classes in line and largely eliminate the concern I posed about porting
changes to both. Still, I'm a bit hesitant, and I'm not actually certain
that this alternative is more intuitive. The name isn't very
Hi Chris, the KIP says it would be a subclass of DefaultReplicationPolicy
that overrides the ReplicationPolicy.offsetSyncsTopic and
ReplicationPolicy.checkpointsTopic. So, not much to maintain and it would
be more intuitive, as you say.
On Wed, Jul 19, 2023, 4:50 PM Chris Egerton wrote:
> HI
HI all,
I'm not sure I understand the benefits of introducing a separate
replication policy class, besides maybe being more readable/intuitive to
users who would want to know when to use one or the other. It feels like
we've swapped out a "fix the bug" property for an entire "fix the bug"
class,
Hi Federico,
I like the idea of implementing `LegacyReplicationPolicy` and avoiding bug
fixes properties. We can drop the idea of the property and just go ahead
with introducing the `LegacyReplicationPolicy` and any user upgrade from
pre-KIP-690 can use this policy instead of default and no impact
Hi, one way to avoid the "fix the bug property" would be to provide
and document an additional LegacyReplicationPolicy, but still keeping
the current DefaultReplicationPolicy as replication.policy.class
default value, which is basically one of the workarounds suggested in
the KIP.
On Tue, Jul 18,
Hi Federico/Omnia,
Generally I like the idea of deprecating and eventually removing "fix the
bug" properties like this, but 4.0 may be a bit soon. I'm also unsure of
how we would instruct users who are relying on the property being set to
"false" to migrate to a version of MM2 that doesn't
Hi Federico,
I don't have any strong opinion one way or the other. The pro of
deprecation is not adding more configs to MM2 as it has too many configs
already. However, we need to think about old MM2 migrating their internal
topics to 4.0 with less impact.
@Chris what do you think?
Cheers
Omnia
Hi Omnia and Chris, I agree with setting
"replication.policy.internal.topic.separator.enabled=true" by default,
but I was wondering if we should also deprecate and remove in Kafka 4.
If there is agreement in having the same behavior for internal and
non-internal topics, then it should be fine, and
Hi Chris, I added a section for backport plan here
https://cwiki.apache.org/confluence/display/KAFKA/KIP-949%3A+Add+flag+to+enable+the+usage+of+topic+separator+in+MM2+DefaultReplicationPolicy#KIP949:AddflagtoenabletheusageoftopicseparatorinMM2DefaultReplicationPolicy-Backportingplan
Cheers,
Hi Omnia,
Yes, I think we ought to state the backport plan in the KIP, since it's
highly unusual for KIP changes or new configuration properties to be
backported and we should get the approval of the community (including
binding and non-binding voters) before implementing it.
Cheers,
Chris
On
Hi Chris,
The implementation should be very small so backporting this to 3.1 and 3.2
would be perfect for this case if you or any other committer are okay with
approving the backporting. Do we need to state this in KIP as well or not?
Also, I’ll open a vote for the KIP today and prepare the pr
Hi Omnia,
Thanks for changing the default, LGTM
As far as backporting goes, we probably won't be doing another release for
3.1, and possibly not for 3.2 either; however, if we can make the
implementation focused enough (which I don't think would be too difficult,
but correct me if I'm wrong),
Hi Chris, thanks for the feedback.
1. regarding the default value I had the same conflict of which version to
break the backward compatibility with. We can just say that this KIP gives
the release Pre KIP-690 the ability to keep the old behaviour with one
config and keep the backwards
Hi Omnia,
Thanks for taking this on! I have some thoughts but the general approach
looks good.
1. Default value
One thing I'm wrestling with is what the default value of the new property
should be. I know on the Jira ticket I proposed that it should be false,
but I'm having second thoughts.
Hi everyone,
I want to start the discussion of the KIP-949. The proposal is here
https://cwiki.apache.org/confluence/display/KAFKA/KIP-949%3A+Add+flag+to+enable+the+usage+of+topic+separator+in+MM2+DefaultReplicationPolicy
19 matches
Mail list logo
