Minor: auto-commit-offset doesn't execute after the deadline#4794
Minor: auto-commit-offset doesn't execute after the deadline#4794chia7712 wants to merge 2 commits intoapache:trunkfrom
Conversation
There was a problem hiding this comment.
Good find. Perhaps to save unneeded calls to system time, we could let invokeCompletedOffsetCommitCallbacks return a boolean to indicate whether any callbacks which invoked?
There was a problem hiding this comment.
Good catch. Will address your comment.
hachikuji
left a comment
hachikuji
left a comment
There was a problem hiding this comment.
Thanks, just a minor question.
| @Test | ||
| public void testAutoCommitAfterInvokingOffsetCommitCallback() throws InterruptedException { | ||
| final List<Long> committedOffset = new ArrayList<>(); | ||
| ConsumerInterceptor ci = new ConsumerInterceptor<Object, Object>() { |
There was a problem hiding this comment.
What's the advantage to doing this in an interceptor instead of the OffsetCommitCallback?
There was a problem hiding this comment.
The ut use interceptor to collect all successful offsets. The OffsetCommitCallback is invoked only once so it can't be substituted for interceptor.
There was a problem hiding this comment.
@hachikuji any comments about the tests? thanks.
|
#5087 had resolved this issue. |
2 participants
one line fix. The callback may take some time to execute so updating the current time is necessary.
Committer Checklist (excluded from commit message)