[GitHub] [kafka] rhauch commented on pull request #11413: KAFKA-13370: add errors when commit offsets failed and add tests

[GitBox <gi...@apache.org>]

rhauch commented on pull request #11413:
URL: https://github.com/apache/kafka/pull/11413#issuecomment-970395759


   @showuon, thanks for trying to fix this issue. But I think the best course of action here is actually to revert #9642, for a few reasons.
   1. It was actually useful to have that boolean `success` parameter within the protected `TaskMetricsGroup.recordCommit(...)` helper method, since it allowed the `WorkerTask.recordCommitSuccess(...)` and `recordCommitFailure(...)` methods to call that one method with desired behavior.
   2. Rolling back is also more encapsulated and significantly easier to backport all the way back to the `2.8` branch, especially since we've significantly refactored the source connector offset commit logic only in `3.0` and later branches.
   
   So I think I'm going to revert #9642, but your additional unit tests here are also very useful. Would you mind creating a new PR with those unit test improvements? Thanks!
   
   


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org