Hello Chris ! Thanks for the quick review ! Good point, task.commit() (existing and proposed) should fail the task on exception.
Currently, if a connector implements task.commit() <https://github.com/apache/kafka/blob/trunk/connect/api/src/main/java/org/apache/kafka/connect/source/SourceTask.java#L117-L119> and throws an exception - this exception will be logged and ignored by AbstractWorkerSourceTask <https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractWorkerSourceTask.java#L581-L587> and will not result in task-failure. Failing the task with "commit()"'s exceptions would be a breaking change (since unhandled exceptions on commit() might start failing the task suddenly after upgrade). In this ticket, I'd like to keep the behaviour consistent with the way AbstractWorkerSourceTask invokes "commit()" currently. However, tasks must fail if commit() throws an exception -> +1. Should we deal with the breaking change in another ticket (targeting the next major release) OR should we try to include the changes in this KIP itself ? What would you suggest? Thanks, Sudesh On Tue, 8 Apr 2025 at 21:02, Chris Egerton <fearthecel...@gmail.com> wrote: > Hi Sudesh, > > Thanks for the KIP. The existing commit method is... not great. I think the > word "futile" you chose in the KIP is putting it lightly. > > The new interface looks clean, implementation seems extremely > straightforward, and everything's backwards compatible. > > The only comment I'll leave is that we should probably document what the > expected behavior is if a task throws from this new method. I'm guessing > the task will fail but let me know if you have other thoughts. > > Cheers, > > Chris > > On Tue, Apr 8, 2025, 11:20 Sudesh Wasnik <wasnik...@gmail.com> wrote: > > > Hi all, > > I would like to discuss KIP-1158. > > > > This KIP introduces a new "commit()" method in KafkaConnect SourceTask > > interface, primarily for use-cases where SourceConnectors need to > > track "source-offset" commits. > > > > Thanks in advance for reviewing ! > > KIP : > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=347933317 > > > > Thanks, > > Sudesh > > >
