Hi Omnia,

Ah, good catch. I think failing to start the checkpoint connector if offset
syncs are disabled is fine. We'd probably want to do that via the
Connector::validate [1] method in order to be able to catch invalid configs
during preflight validation, but it's not necessary to get that specific in
the KIP (especially since we may add other checks as well).

[1] -
https://kafka.apache.org/37/javadoc/org/apache/kafka/connect/connector/Connector.html#validate(java.util.Map)

Cheers,

Chris

On Thu, Apr 4, 2024 at 8:07 PM Omnia Ibrahim <o.g.h.ibra...@gmail.com>
wrote:

> Thanks Chris for the feedback
> > 1. It'd be nice to mention that increasing the max offset lag to INT_MAX
> > could work as a partial workaround for users on existing versions (though
> > of course this wouldn't prevent creation of the syncs topic).
> I updated the KIP
>
> > 2. Will it be illegal to disable offset syncs if other features that rely
> > on offset syncs are explicitly enabled in the connector config? If
> they're
> > not explicitly enabled then it should probably be fine to silently
> disable
> > them, but I'd be interested in your thoughts.
> The rest of the features that relays on this is controlled by
> emit.checkpoints.enabled (enabled by default) and
> sync.group.offsets.enabled (disabled by default) which are part of
> MirrorCheckpointConnector config not MirrorSourceConnector, I was thinking
> that MirrorCheckpointConnector should fail to start if
> emit.offset-syncs.enabled is disabled while emit.checkpoints.enabled and/or
> sync.group.offsets.enabled are enabled as no point of creating this
> connector if the main part is disabled. WDYT?
>
> Thanks
> Omnia
>
> > On 3 Apr 2024, at 12:45, Chris Egerton <fearthecel...@gmail.com> wrote:
> >
> > Hi Omnia,
> >
> > Thanks for the KIP! Two small things come to mind:
> >
> > 1. It'd be nice to mention that increasing the max offset lag to INT_MAX
> > could work as a partial workaround for users on existing versions (though
> > of course this wouldn't prevent creation of the syncs topic).
> >
> > 2. Will it be illegal to disable offset syncs if other features that rely
> > on offset syncs are explicitly enabled in the connector config? If
> they're
> > not explicitly enabled then it should probably be fine to silently
> disable
> > them, but I'd be interested in your thoughts.
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, Apr 3, 2024, 20:41 Luke Chen <show...@gmail.com> wrote:
> >
> >> Hi Omnia,
> >>
> >> Thanks for the KIP!
> >> It LGTM!
> >> But I'm not an expert of MM2, it would be good to see if there is any
> other
> >> comment from MM2 experts.
> >>
> >> Thanks.
> >> Luke
> >>
> >> On Thu, Mar 14, 2024 at 6:08 PM Omnia Ibrahim <o.g.h.ibra...@gmail.com>
> >> wrote:
> >>
> >>> Hi everyone, I would like to start a discussion thread for KIP-1031:
> >>> Control offset translation in MirrorSourceConnector
> >>>
> >>>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1031%3A+Control+offset+translation+in+MirrorSourceConnector
> >>>
> >>> Thanks
> >>> Omnia
> >>>
> >>
>
>

Reply via email to