Hi Omnia, Thank you for the reply, it makes sense.
A couple more comments: 1) I'm assuming the new interface and default implementation will be in the mirror-client project? as the names of some of these topics are needed by RemoteClusterUtils on the client-side. 2) I'm about to open a KIP to specify where the offset-syncs topic is created by MM2. In restricted environments, we'd prefer MM2 to only have read access to the source cluster and have the offset-syncs on the target cluster. I think allowing to specify the cluster where to create that topic would be a natural extension of the interface you propose here. So I wonder if your interface could be named InternalTopicsPolicy? That's a bit more generic than InternalTopicNamingPolicy. That would also match the configuration setting, internal.topics.policy.class, you're proposing. Thanks On Thu, Dec 3, 2020 at 10:15 PM Omnia Ibrahim <o.g.h.ibra...@gmail.com> wrote: > > Hi Mickael, > Thanks for your feedback! > Regards your question about having more configurations, I considered adding > configuration per each topic however this meant adding more configurations > for MM2 which already have so many, also the more complicated and advanced > replication pattern you have between clusters the more configuration lines > will be added to your MM2 config which isn't going to be pretty if you > don't have the same topics names across your clusters. > > Also, it added more complexity to the implementation as MM2 need to > 1- identify if a topic is checkpoints so we could list the checkpoints > topics in MirrorMaker 2 utils as one cluster could have X numbers > checkpoints topics if it's connected to X clusters, this is done right now > by listing any topic with suffix `.checkpoints.internal`. This could be > done by add `checkpoints.topic.suffix` config but this would make an > assumption that checkpoints will always have a suffix also having a suffix > means that we may need a separator as well to concatenate this suffix with > a prefix to identify source cluster name. > 2- identify if a topic is internal, so it shouldn't be replicated or track > checkpoints for it, right now this is relaying on disallow topics with > `.internal` suffix to be not replicated and not tracked in checkpoints but > with making topics configurable we need a way to define what is an internal > topic. This could be done by making using a list of all internal topics > have been entered to the configuration. > > So having an interface seemed easier and also give more flexibility for > users to define their own topics name, define what is internal topic means, > how to find checkpoints topics and it will be one line config for each > herder, also it more consistence with MM2 code as MM2 config have > TopicFilter, ReplicationPolicy, GroupFilter, etc as interface and they can > be overridden by providing a custom implementation for them or have some > config that change their default implementations. > > Hope this answer your question. I also updated the KIP to add this to the > rejected solutions. > > > On Thu, Dec 3, 2020 at 3:19 PM Mickael Maison <mickael.mai...@gmail.com> > wrote: > > > Hi Omnia, > > > > Thanks for the KIP. Indeed being able to configure MM2's internal > > topic names would be a nice improvement. > > > > Looking at the KIP, I was surprised you propose an interface to allow > > users to specify names. Have you considered making names changeable > > via configurations? If so, we should definitely mention it in the > > rejected alternatives as it's the first method that comes to mind. > > > > I understand an interface gives a lot of flexibility but I'd expect > > topic names to be relatively simple and known in advance in most > > cases. > > > > I've not checked all use cases but something like below felt appropriate: > > clusters = primary,backup > > primary->backup.offsets-sync.topic=backup.mytopic-offsets-sync > > > > On Tue, Dec 1, 2020 at 3:36 PM Omnia Ibrahim <o.g.h.ibra...@gmail.com> > > wrote: > > > > > > Hey everyone, > > > Please take a look at KIP-690: > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-690%3A+Add+additional+configuration+to+control+MirrorMaker+2+internal+topics+naming+convention > > > > > > Thanks for your feedback and support. > > > > > > Omnia > > > > >
