C0urante commented on a change in pull request #10528:
URL: https://github.com/apache/kafka/pull/10528#discussion_r614207888
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/SourceTaskOffsetCommitter.java
##########
@@ -105,6 +105,11 @@ public void remove(ConnectorTaskId id) {
}
private void commit(WorkerSourceTask workerTask) {
+ if (!workerTask.shouldCommitOffsets()) {
Review comment:
I initially wanted to do just that, but held off for two reasons:
1. The `SourceTaskOffsetCommitter` log message may be valuable to some
users, especially if they've muted the `WorkerSourceTask` namespace or at least
raised its level to `INFO`.
2. It's a little easier to test this logic separately by creating a separate
method that can be queried on its own. The `boolean` return value of
`WorkerSourceTask::commitOffsets` makes it difficult to determine if an offset
commit has been attempted and failed, or skipped entirely.
I'm not super attached to either approach; if you have thoughts on why one
or the other is better I'm happy to hear them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
