Hi Mickael, Did you get time to review the changes to the KIP? If you okay with it could you vote for the KIP here ttps://www.mail-archive.com/dev@kafka.apache.org/msg113575.html <https://www.mail-archive.com/dev@kafka.apache.org/msg113575.html>? Thanks
On Thu, Dec 10, 2020 at 2:19 PM Omnia Ibrahim <o.g.h.ibra...@gmail.com> wrote: > 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 >> > > > >> > > >> >
