Hi Chris,
Thanks, that makes sense - I hadn't considered the case where the worker
itself becomes a zombie.
Thanks,
Yash
On Wed, Apr 12, 2023 at 10:40 PM Chris Egerton <chr...@aiven.io.invalid>
wrote:
> Hi Yash,
>
> Great, we can use the transactional ID for task zero for now 👍
>
> As far as why: we'd need to fence out that producer in the event that tasks
> for the connector are brought up while the alter offsets request is still
> taking place, since we'd want to make sure that the offsets aren't altered
> after tasks are brought up. I think it may be possible right now in
> extremely niche circumstances where the worker servicing the alter offsets
> request loses leadership of the cluster. More realistically, I think it
> leaves room for tweaking the logic for handling these requests to not use a
> five-second timeout in the future (we could potentially just
> fire-and-forget the request and, if new tasks are started for the connector
> in the meantime, trust that the request will get automatically fenced out
> without doing any more work).
>
> Cheers,
>
> Chris
>
> On Wed, Apr 12, 2023 at 1:01 PM Yash Mayya <yash.ma...@gmail.com> wrote:
>
> > Hi Chris and Greg,
> >
> > The current implementation does already use the transactional ID for
> task 0
> > so no complaints from me. Although I'm not sure I follow the concerns
> w.r.t
> > zombie fencing? In which cases would we need to fence out the
> transactional
> > producer instantiated for altering offsets?
> >
> > Thanks,
> > Yash
> >
> > On Wed, Apr 12, 2023 at 9:02 PM Chris Egerton <chr...@aiven.io> wrote:
> >
> > > Hi Greg,
> > >
> > > I hadn't considered the implications W/R/T zombie fencing. I agree that
> > > using the transactional ID for task 0 is better in that case.
> > >
> > > Yash (who is implementing this part, cc'd), does this seem reasonable
> to
> > > you?
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Tue, Apr 11, 2023 at 3:23 PM Greg Harris
> <greg.har...@aiven.io.invalid
> > >
> > > wrote:
> > >
> > >> Chris & Yash,
> > >>
> > >> 1. Since the global offsets topic does not have transactions on it
> > >> already,
> > >> I don't think adding transactions just for these reset operations
> would
> > be
> > >> an improvement. The transactional produce would not exclude other
> > >> non-transactional producers, but hanging transactions on the global
> > >> offsets
> > >> topic would negatively impact the general cluster health. Your
> proposed
> > >> strategy seems reasonable to me.
> > >>
> > >> 2. While it may be the connector performing the offset reset and not
> the
> > >> task, I think it would be preferable for the connector to use task 0's
> > >> task-id and 'impersonate' the task for the purpose of changing the
> > >> offsets.
> > >> I think the complication elsewhere (getting users to provide a new
> ACL,
> > >> expanding fencing to also fence the connector transaction id, etc) is
> > not
> > >> practically worth it to change 1 string value in the logs.
> > >> I would find a separate transaction ID beneficial if the connector
> could
> > >> be
> > >> given a different principal from the task, and be given distinct ACLs.
> > >> However, I don't think this is possible or desirable, and so I don't
> > think
> > >> it's relevant right now. Let me know if there are any other ways that
> > the
> > >> connector transaction ID would be useful.
> > >>
> > >> Thanks for all the effort on this feature!
> > >> Greg
> > >>
> > >> On Tue, Apr 11, 2023 at 7:52 AM Chris Egerton <chr...@aiven.io.invalid
> >
> > >> wrote:
> > >>
> > >> > Hi all,
> > >> >
> > >> > A couple slight tweaks to the design have been proposed during
> > >> > implementation and I'd like to report them here to make sure that
> > >> they're
> > >> > acceptable to all who previously voted for this KIP. I've updated
> the
> > >> KIP
> > >> > to include these changes but will be happy to revert and/or amend if
> > >> there
> > >> > are any concerns.
> > >> >
> > >> > 1. We would like to refrain from using a transaction when resetting
> > >> source
> > >> > connector offsets in the worker's global offsets topic when
> > exactly-once
> > >> > support is enabled. We would continue to use a transaction when
> > >> resetting
> > >> > offsets in the connector's offsets topic. Discussed in [1].
> > >> >
> > >> > 2. We would like to use a transactional ID of
> ${groupId}-${connector}
> > to
> > >> > alter/reset source connector offsets when exactly-once support is
> > >> enabled,
> > >> > where ${groupId} is the group ID of the Connect cluster and
> > >> ${connector} is
> > >> > the name of the connector. This is raised here because it would
> > >> introduce
> > >> > an additional ACL requirement for this API. A less-elegant
> alternative
> > >> that
> > >> > would obviate the additional ACL requirement is to use the
> > >> transactional ID
> > >> > that would be used by task 0 of the connector, but this may be
> > >> confusing to
> > >> > users as it could indicate that the task is actually running.
> > Discussed
> > >> in
> > >> > [2].
> > >> >
> > >> > [1] -
> > >> https://github.com/apache/kafka/pull/13465/#issuecomment-1486718538
> > >> > [2] -
> > >> https://github.com/apache/kafka/pull/13465/#discussion_r1159694956
> > >> >
> > >> > Cheers,
> > >> >
> > >> > Chris
> > >> >
> > >> > On Fri, Mar 3, 2023 at 10:22 AM Chris Egerton <chr...@aiven.io>
> > wrote:
> > >> >
> > >> > > Hi all,
> > >> > >
> > >> > > Thanks for the votes! I'll cast a final +1 myself and close the
> vote
> > >> out.
> > >> > >
> > >> > > This KIP passes with the following +1 votes (and no +0 or -1
> votes):
> > >> > >
> > >> > > • Greg Harris
> > >> > > • Yash Mayya
> > >> > > • Knowles Atchison Jr
> > >> > > • Mickael Maison (binding)
> > >> > > • Tom Bentley (binding)
> > >> > > • Josep Prat (binding)
> > >> > > • Chris Egerton (binding, author)
> > >> > >
> > >> > > I'll write up Jira tickets and begin implementing things next
> week.
> > >> > >
> > >> > > Cheers,
> > >> > >
> > >> > > Chris
> > >> > >
> > >> > > On Fri, Mar 3, 2023 at 10:07 AM Josep Prat
> > >> <josep.p...@aiven.io.invalid>
> > >> > > wrote:
> > >> > >
> > >> > >> Hi Chris,
> > >> > >>
> > >> > >> Thanks for the KIP. I have a non-blocking comment on the DISCUSS
> > >> thread.
> > >> > >>
> > >> > >> +1 (binding).
> > >> > >>
> > >> > >> Best,
> > >> > >>
> > >> > >> On Wed, Mar 1, 2023 at 12:16 PM Tom Bentley <tbent...@redhat.com
> >
> > >> > wrote:
> > >> > >>
> > >> > >> > Hi Chris,
> > >> > >> >
> > >> > >> > Thanks for the KIP.
> > >> > >> >
> > >> > >> > +1 (binding).
> > >> > >> >
> > >> > >> > Cheers,
> > >> > >> >
> > >> > >> > Tom
> > >> > >> >
> > >> > >> > On Wed, 15 Feb 2023 at 16:11, Chris Egerton
> > >> <chr...@aiven.io.invalid>
> > >> > >> > wrote:
> > >> > >> >
> > >> > >> > > Hi all,
> > >> > >> > >
> > >> > >> > > Thanks to everyone who's voted so far! Just wanted to bump
> this
> > >> > thread
> > >> > >> > and
> > >> > >> > > see if we could get a few more votes; currently we're at +3
> > >> > >> non-binding
> > >> > >> > > and +1 binding. Hoping we can get this approved, reviewed,
> and
> > >> > merged
> > >> > >> in
> > >> > >> > > time for 3.5.0.
> > >> > >> > >
> > >> > >> > > Cheers,
> > >> > >> > >
> > >> > >> > > Chris
> > >> > >> > >
> > >> > >> > > On Tue, Jan 31, 2023 at 2:52 AM Mickael Maison <
> > >> > >> mickael.mai...@gmail.com
> > >> > >> > >
> > >> > >> > > wrote:
> > >> > >> > >
> > >> > >> > > > Thanks Chris for the KIP, this is a much needed feature!
> > >> > >> > > >
> > >> > >> > > > +1 (binding)
> > >> > >> > > >
> > >> > >> > > >
> > >> > >> > > > On Tue, Jan 24, 2023 at 3:45 PM Knowles Atchison Jr
> > >> > >> > > > <katchiso...@gmail.com> wrote:
> > >> > >> > > > >
> > >> > >> > > > > +1 (non binding)
> > >> > >> > > > >
> > >> > >> > > > > On Tue, Jan 24, 2023 at 5:24 AM Yash Mayya <
> > >> > yash.ma...@gmail.com>
> > >> > >> > > wrote:
> > >> > >> > > > >
> > >> > >> > > > > > Hi Chris,
> > >> > >> > > > > >
> > >> > >> > > > > > I'm +1 (non-binding). Thanks again for proposing this
> > >> > extremely
> > >> > >> > > > > > valuable addition to Kafka Connect!
> > >> > >> > > > > >
> > >> > >> > > > > > Thanks,
> > >> > >> > > > > > Yash
> > >> > >> > > > > >
> > >> > >> > > > > > On Thu, Jan 19, 2023 at 12:11 AM Chris Egerton
> > >> > >> > > <chr...@aiven.io.invalid
> > >> > >> > > > >
> > >> > >> > > > > > wrote:
> > >> > >> > > > > >
> > >> > >> > > > > > > Hi all,
> > >> > >> > > > > > >
> > >> > >> > > > > > > I'd like to call for a vote on KIP-875, which adds
> > >> support
> > >> > for
> > >> > >> > > > viewing
> > >> > >> > > > > > and
> > >> > >> > > > > > > manipulating the offsets of connectors to the Kafka
> > >> Connect
> > >> > >> REST
> > >> > >> > > API.
> > >> > >> > > > > > >
> > >> > >> > > > > > > The KIP:
> > >> > >> > > > > > >
> > >> > >> > > > > > >
> > >> > >> > > > > >
> > >> > >> > > >
> > >> > >> > >
> > >> > >> >
> > >> > >>
> > >> >
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect
> > >> > >> > > > > > >
> > >> > >> > > > > > > The discussion thread:
> > >> > >> > > > > > >
> > >> > >> https://lists.apache.org/thread/m5bklnh5w4mwr9nbzrmfk0pftpxfjd02
> > >> > >> > > > > > >
> > >> > >> > > > > > > Cheers,
> > >> > >> > > > > > >
> > >> > >> > > > > > > Chris
> > >> > >> > > > > > >
> > >> > >> > > > > >
> > >> > >> > > >
> > >> > >> > >
> > >> > >> >
> > >> > >>
> > >> > >>
> > >> > >> --
> > >> > >> [image: Aiven] <https://www.aiven.io>
> > >> > >>
> > >> > >> *Josep Prat*
> > >> > >> Open Source Engineering Director, *Aiven*
> > >> > >> josep.p...@aiven.io | +491715557497
> > >> > >> aiven.io <https://www.aiven.io> | <
> > >> > >> https://www.facebook.com/aivencloud>
> > >> > >> <https://www.linkedin.com/company/aiven/> <
> > >> > >> https://twitter.com/aiven_io>
> > >> > >> *Aiven Deutschland GmbH*
> > >> > >> Alexanderufer 3-7, 10117 Berlin
> > >> > >> Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen
> > >> > >> Amtsgericht Charlottenburg, HRB 209739 B
> > >> > >>
> > >> > >
> > >> >
> > >>
> > >
> >
>
