Hi all, Regarding Omnia's last point: I also second that with a reasonable default we won't really complicate average users' problems while setting a config is simpler than having to provide a jar to override the replication policy. I have also mulled over what if we created a separate replication policy to capture this and KIP-1074 <https://cwiki.apache.org/confluence/display/KAFKA/KIP-1074> together but unfortunately that'd further complicate the problem as we wouldn't have to just set a few configs but additionally we could do it if we also configured a new replication policy.
Regarding long term (not in the scope of this KIP), I think the best would be to deprecate ReplicationPolicy as an interface and just replace it with a fully configurable DefaultReplicationPolicy class where all topic names, patterns, separators and filters can be changed. We wouldn't lose functionality in IdentityReplicationPolicy as it currently doesn't redefine topic names and all that but I don't think that we need an interface here either. Best, Viktor On Wed, Jul 31, 2024 at 5:12 PM Kondrát Bertalan <kb.p...@gmail.com> wrote: > Hi All, > > *Viktor * > I was wrong sorry, Chris is right we have the per replication flow > configuration for free. > > *Chris, regarding your comments.* > *>IMO we don't need the "default." prefix* > Sure I can do that, my intention with that was to make it clear for the > users, that this property is not compatible with anything else but the > DefaultReplicationPolicy > > *>I'm also a little hazy on the motivation for the change. Just out of* > > > > *curiosity, what exactly is meant by "the "heartbeats" topics of > othersystems" in the Jira ticket's description? Are we trying to > betteraccommodate cases where other harder-to-configure systems (like a > pickysource connector, for example) * > Exactly, we had a customer with a MongoDb connector if I recall it > correctly, and for some reason they could not change the topic name on > connector side. > > *Omania* > You are right, I did not notice that my solution is in the rejected > alternatives of KIP-690. I can't argue with your point, and I understand > that the configuration of the mirror maker is not straight forward with all > that options. But those properties are hidden from the user with a > reasonable default value, so in my opinion it should not confuse the users, > but it is there if someone want to modify it. > Renaming an internal topic should not require implementing a new > replication policy class and attaching it to the classpath in my opinion. > I let you and the community decide whether is it worth to reconsider the > previous solution. (I have updated the KIP) > > Thanks > Berci > > On Fri, Feb 2, 2024 at 4:31 PM Omnia Ibrahim <o.g.h.ibra...@gmail.com> > wrote: > > > I noticed you have something addressed in the rejected alternatives in > > your KIP regarding KIP-690, however, as the KIP is implementing one of > the > > previous rejected alternatives for KIP-690 I think it would be nice to > > update the motivation section in the new KIP with some clarification for > > why what was mentioned in the rejected alternatives in KIP-690 isn’t > valid > > anymore. > > > > > On 2 Feb 2024, at 15:23, Omnia Ibrahim <o.g.h.ibra...@gmail.com> > wrote: > > > > > > Hi > > > Thanks for the KIP. I don’t know if you had a look before into the > > rejection alternatives in KIP-690 > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-690%3A+Add+additional+configuration+to+control+MirrorMaker+2+internal+topics+naming+convention#KIP690:AddadditionalconfigurationtocontrolMirrorMaker2internaltopicsnamingconvention-RejectedAlternatives > > or not but one of the things we were worried about is the amount of > > configuration MM2 already has at the moment specially for advanced and > > complicated replication between multiple clusters. This why we > bin-backing > > on the existing of replication policy configs to allow users to define > > their own replication policy with their customised internal topics. > > > > > > Is the assumption now different meaning that we are okay to add more > > configs for mm2? If so can you mention this in the KIP as it was rejected > > before in another KIP > > > > > > Also wouldn’t this mean that we now have two different ways to set the > > `heartbeat` topic name one by this config and another one by overriding > the > > replication policy class? It maybe worth mentioning in your KIP why you > are > > adding this other path to configure heartbeat topic with single config > > instead of the existing route which providing replication policy with the > > new names of internal topics. And when users should opt-in for each one > of > > them. > > > > > > Thanks > > > > > >> On 22 Jan 2024, at 19:52, Chris Egerton <chr...@aiven.io.INVALID> > > wrote: > > >> > > >> Hi Berci, > > >> > > >> Thanks for the KIP! > > >> > > >> IMO we don't need the "default." prefix for the new property, and it > > >> deviates a bit from the precedent set by properties like > > >> "replication.policy.internal.topic.separator.enabled". I think we can > > just > > >> call it "replication.policy.heartbeats.topic", or if we really want to > > be > > >> precise, "replication.policy.heartbeats.topic.name". > > >> > > >> Regarding multiple source->target pairs, won't we get support for this > > for > > >> free if we add the new property to the DefaultReplicationPolicy class? > > IIRC > > >> it's already possible to configure replication policies on a > > >> per-replication-flow basis with that syntax, I don't see why this > > wouldn't > > >> be the case for the new property. > > >> > > >> I'm also a little hazy on the motivation for the change. Just out of > > >> curiosity, what exactly is meant by "the "heartbeats" topics of other > > >> systems" in the Jira ticket's description? Are we trying to better > > >> accommodate cases where other harder-to-configure systems (like a > picky > > >> source connector, for example) create and use a "heartbeats" topic, or > > are > > >> we trying to enable multiple MM2 heartbeat connectors to target the > same > > >> Kafka cluster? I can understand the former as a niche but possible > > scenario > > >> and one that we can make room for, but the latter is a bit harder to > > >> justify short of, e.g., fine-tuning the heartbeat emission interval > > based > > >> on the eventual target of the replication flow that will be reading > from > > >> the heartbeats topic. > > >> > > >> I don't raise the above to cast doubt on the KIP, really I'm just > > curious > > >> about how people are using MM2. > > >> > > >> Cheers, > > >> > > >> Chris > > >> > > >> On Thu, Jan 18, 2024 at 6:11 AM Kondrát Bertalan <kb.p...@gmail.com> > > wrote: > > >> > > >>> Hi Viktor, > > >>> > > >>> Let me address your points one by one. > > >>> > > >>> 1. The current implementation does not support the source->target > > pair > > >>> based configuration, it is global. > > >>> 2. Yes, I introduced that property both in the client and in the > > >>> connectors > > >>> 3. This is a great idea, I am going to do that, and also I tried to > > >>> construct the property name in a way that makes this clear for the > > >>> users: ' > > >>> default.replication.policy.heartbeats.topic.name' > > >>> 4. Yeah, that was my impression too. > > >>> > > >>> Thanks, > > >>> Berci > > >>> > > >>> On Wed, Jan 17, 2024 at 4:51 PM Viktor Somogyi-Vass > > >>> <viktor.somo...@cloudera.com.invalid> wrote: > > >>> > > >>>> Hi Bertalan, > > >>>> > > >>>> Thanks for creating this KIP. > > >>>> A couple of observations/questions: > > >>>> 1. If I have multiple source->target pairs, can I set this property > > per > > >>>> cluster by prefixing with "source->target" as many other configs or > > is it > > >>>> global? > > >>>> 2. The replication policy must be set in MirrorClient as well. Is > your > > >>>> change applicable to both MirrorClient and the connectors as well? > > >>>> 3. It might be worth pointing out (both in the docs and the KIP) > that > > if > > >>>> the user overrides the replication policy to any other than > > >>>> DefaultReplicationPolicy, then this config has no effect. > > >>>> 4. With regards to integration tests, I tend to lean towards that we > > >>> don't > > >>>> need them if we can cover this well with unit tests and mocking. > > >>>> > > >>>> Thanks, > > >>>> Viktor > > >>>> > > >>>> On Wed, Jan 17, 2024 at 12:23 AM Ryanne Dolan < > ryannedo...@gmail.com> > > >>>> wrote: > > >>>> > > >>>>> Makes sense to me, +1. > > >>>>> > > >>>>> On Tue, Jan 16, 2024 at 5:04 PM Kondrát Bertalan < > kb.p...@gmail.com> > > >>>>> wrote: > > >>>>> > > >>>>>> Hey Team, > > >>>>>> > > >>>>>> I would like to start a discussion thread about the *KIP-1016 Make > > MM2 > > >>>>>> heartbeats topic name configurable > > >>>>>> < > > >>>>>> > > >>>> > > >>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1016+Make+MM2+heartbeats+topic+name+configurable > > >>>>>>> * > > >>>>>> . > > >>>>>> > > >>>>>> This KIP aims to make the default heartbeat topic name > > (`heartbeats`) > > >>> in > > >>>>>> the DefaultReplicationPolicy configurable via a property. > > >>>>>> Since this is my first KIP and the change is small, I implemented > it > > >>> in > > >>>>>> advance so, I can include the PR > > >>>>>> <https://github.com/apache/kafka/pull/15200> as well. > > >>>>>> > > >>>>>> I appreciate all your feedbacks and comments. > > >>>>>> > > >>>>>> Special thanks to Viktor Somogyi-Vass < > viktor.somo...@cloudera.com> > > >>> and > > >>>>>> Daniel > > >>>>>> Urban <urb.dani...@gmail.com> for the original idea and help. > > >>>>>> Thank you, > > >>>>>> Berci > > >>>>>> > > >>>>>> -- > > >>>>>> *Bertalan Kondrat* | Founder, SWE > > >>>>>> servy.hu <https://www.servy.hu/> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> <https://www.cloudera.com/> > > >>>>>> ------------------------------ > > >>>>>> > > >>>>> > > >>>> > > >>> > > >>> > > >>> -- > > >>> *Bertalan Kondrat* | Founder > > >>> t. +36(70) 413-4801 > > >>> servy.hu <https://www.servy.hu/> > > >>> > > >>> > > >>> [image: Servy] <https://www.cloudera.com/> > > >>> ------------------------------ > > >>> > > > > > > > > > -- > *Bertalan Kondrat* | Founder > t. +36(70) 413-4801 > servy.hu <https://www.servy.hu/> > > > [image: Servy] <https://www.cloudera.com/> > ------------------------------ >
