Hi Mickael, 1) That's right the interface and default implementation will in mirror-connect 2) Renaming the interface should be fine too especially if you planning to move other functionality related to the creation there, I can edit this
if you are okay with that please vote for the KIP here https://www.mail-archive.com/dev@kafka.apache.org/msg113575.html Thanks Omnia On Thu, Dec 10, 2020 at 12:58 PM Mickael Maison <mickael.mai...@gmail.com> wrote: > 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 > > > > > > > >
