Skip to content

KAFKA-15091: Fix misleading Javadoc for SourceTask::commit #13948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
C0urante merged 2 commits into from
Jul 18, 2023
Merged

KAFKA-15091: Fix misleading Javadoc for SourceTask::commit #13948

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01f6e84
KAFKA-15091: Fix misleading Javadoc for SourceTask::commit
yashmayya Jul 1, 2023
37232c3
Remove difference between EoS and non-EoS; add reference to SourceTas…
yashmayya Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions connect/api/src/main/java/org/apache/kafka/connect/source/SourceTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ public void initialize(SourceTaskContext context) {
public abstract List<SourceRecord> poll() throws InterruptedException;

/**
* <p>
* Commit the offsets, up to the offsets that have been returned by {@link #poll()}. This
* method should block until the commit is complete.
* This method is invoked periodically when offsets are committed for this source task. Note that the offsets
* being committed won't necessarily correspond to the latest offsets returned by this source task via
* {@link #poll()}. Also see {@link #commitRecord(SourceRecord, RecordMetadata)} which allows for a more
* fine-grained tracking of records that have been successfully delivered.
* <p>
* SourceTasks are not required to implement this functionality; Kafka Connect will record offsets
* automatically. This hook is provided for systems that also need to store offsets internally

@yashmayya yashmayya Jul 1, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on the fence on whether or not to remove this bit as well since I can't think of a realistic use-case where this method would help with offset tracking in the source system in its current form (and hence the question on deprecation of this method in https://issues.apache.org/jira/browse/KAFKA-15091).

@C0urante C0urante Jul 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "store offsets internally" language isn't great, but I'd rather leave it for now and explore it further if/when we start discussing deprecating this method. Connector developers might theoretically use this for acknowledging JMS records, for example, which in a very loose sense is storing offsets (or at least, some JMS-specific equivalent of them) in that system.

Expand Down