Skip to content

KAFKA-14220: Update WorkerSinkTask.java#12622

Closed
ghost wants to merge 1 commit intotrunkfrom
unknown repository
Closed

KAFKA-14220: Update WorkerSinkTask.java#12622
ghost wants to merge 1 commit intotrunkfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 12, 2022

No description provided.

@C0urante C0urante self-requested a review September 13, 2022 12:55
@C0urante
Copy link
Copy Markdown
Contributor

Thanks @kumarpritam863, this is an important fix. I did some research on the history behind incremental consumer rebalancing and how it relates to the rebalance listener API, and found that this PR contained some great details, including an explicit confirmation on the expected ordering of callback invocation:

The ordering of the callback would be the following:
a. Callback onPartitionsRevoked / onPartitionsLost triggered.
b. Update the assignment (both revoked and added).
c. Callback onPartitionsAssigned triggered.

One case I'm wondering about is when the task fails, which would cause its consumer to leave the group. I believe that this would cause onPartitionsRevoked to be invoked in the rebalance listener, but not onPartitionsAssigned, so the metrics for the task (which are still available for failed tasks until they are explicitly revoked from the worker) would become inaccurate.

Could we forcibly reset the partition count metric to zero in WorkerSinkTask::close to address this case? (Probably with a brief comment explaining why we have this special logic).

It'd also be nice to see some tests for this logic, both to verify correctness and help prevent regression.

@ghost ghost closed this Sep 14, 2022
@ghost ghost deleted the KAFKA-14220 branch September 14, 2022 09:14
@C0urante C0urante changed the title Update WorkerSinkTask.java KAFKA-14220: Update WorkerSinkTask.java Sep 15, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant