KAFKA-8179: Part 3, Add PartitionsLost API for resetGenerations and metadata/subscription change by guozhangwang · Pull Request #6884 · apache/kafka

guozhangwang · 2019-06-05T01:48:10Z

Add onPartitionsLost into the RebalanceListener, which will be triggered when a) reset generations due to errors, and b) topic metadata changed and owned partitions no longer exist.

When resetting generations proactively and subscription changes, we will trigger onPartitionsRevoked instead.

Semantical behavior change: with COOPERATIVE protocol, if the revoked / lost partitions are empty, do not trigger the corresponding callback at all. For added partitions though, even if it is empty we would still trigger the callback as a way to notify the rebalance event; with EAGER protocol, revoked / assigned callbacks are always triggered.

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.

In this way we are assured that users can still access the partitions being revoked, whereas they can also access the partitions being added.

Semantical behavior change (KAFKA-4600): if the rebalance listener throws an exception, pass it along all the way to the consumer.poll caller, but still completes the rest of the actions. Also, the newly assigned partitions list does not gets affected with exception thrown since it is just for notifying the users.
Semantical behavior change: the ConsumerCoordinator would not try to modify assignor's returned assignments, instead it will validate that assignments and set the error code accordingly: if there are overlaps between added / revoked partitions, it is a fatal error and would be communicated to all members to stop; if revoked is not empty, it is an error indicate re-join; otherwise, it is normal.
Minor: with the error code removed from the Assignment, ConsumerCoordinator will request re-join if the revoked partitions list is not empty.
Updated ConsumerCoordinatorTest accordingly. Also found a minor bug in MetadataUpdate that removed topic would still be retained with null value of num.partitions.
Updated a few other unit tests that are exposed due to this change.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

…-client-algorithm

…pache#6511) As described in KIP-443 (https://cwiki.apache.org/confluence/display/KAFKA/KIP-443%3A+Return+to+default+segment.ms+and+segment.index.bytes+in+Streams+repartition+topics). We want to remove the aggressive overrides of segment.ms and segment.index.bytes for repartition topics. The remaining segment.bytes should still be effective in bounding its footprint. Reviewers: Bill Bejeck <bbejeck@gmail.com>

…-consumer-protocol

…-emigrate-callback

guozhangwang · 2019-08-06T23:01:46Z

triggered system tests for client: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2854/

for streams: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2855/

ableegoldman

Thanks for kicking off system tests -- everything LGTM

hachikuji · 2019-08-07T00:44:53Z

     * Unsubscribe from topics currently subscribed with {@link #subscribe(Collection)} or {@link #subscribe(Pattern)}.
     * This also clears any partitions directly assigned through {@link #assign(Collection)}.
+     *
+     * @throws org.apache.kafka.common.KafkaException for any other unrecoverable errors (e.g. rebalance callback errors)


It might be worth mentioning in the upgrade notes the fact that we now invoke revocation logic in unsubscribe and close.

Yes, that's in my plan -- I have another PR on web docs for the changes related, but wanted to keep this one from continue growing.

hachikuji · 2019-08-07T00:58:14Z

+        // heartbeat error handling is executed by heartbeat thread, in which case
+        // we would not drop partitions and trigger rebalance callback as it should
+        // only be triggered by the caller thread
+        if (api != ApiKeys.HEARTBEAT) {


Hmm.. I am not sure this is sufficient. Any of the responses could return from the heartbeat thread.

guozhangwang · 2019-08-07T05:20:08Z

Re-triggered https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2857/ and https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2858/

guozhangwang · 2019-08-07T15:11:45Z

Jenkins failures are irrelevant.

guozhangwang · 2019-08-07T16:59:31Z

retest this please

hachikuji · 2019-08-08T00:16:04Z

-                    .compose(new LeaveGroupResponseHandler());
+        }
+
+        // we need to reset generation first in order to trigger the rebalance callback if necessary, before sending


This seems no longer relevant?

guozhangwang · 2019-08-08T05:42:06Z

Addressed latest comment.

guozhangwang · 2019-08-08T16:14:34Z

Jenkins failures are irrelevant and non-overlapping.

hachikuji

LGTM. I left a suggestion on an alternative name for onLeaveGroup.

hachikuji · 2019-08-08T21:06:26Z

+    /**
+     * Invoked prior to each leave group event. This is typically used to cleanup assigned partitions
+     */
+    protected void onLeaveGroup() {}


For consistency with onJoinPrepare and onJoinComplete, would it be reasonable to call this onLeavePrepare?

Of course :)

…-emigrate-callback

* apache-github/trunk: MINOR: Ignore dynamic log4j log level tests (apache#7183) KAFKA-8748: Fix flaky testDescribeLogDirsRequest (apache#7182) KAFKA-8598: Use automatic RPC generation in RenewDelegationToken KAFKA-8179: Part 3, Add PartitionsLost API for resetGenerations and metadata/subscription change (apache#6884)

…setGenerations and metadata/subscription change (apache#6884)" This reverts commit e867a58.

This is a cherry-pick of the bug-fix included in #6884 to 2.3 and older branch. Reviewers: A. Sophie Blee-Goldman <sophie@confluent.io>, Matthias J. Sax <mjsax@apache.org>

guozhangwang added 30 commits April 1, 2019 16:20

bump up consumer protocol to v2

4cf89e6

update consumer coordinator based on rebalance protocol

094ab43

Merge branch 'trunk' of https://github.com/apache/kafka into K8179-p2…

368f2bc

…-client-algorithm

minor fix

cb9fb49

set re-join error code

f7ab5e2

add emigrate callback

347ca5f

typo fix

d3af860

github comments

cb25f6c

rebase from p1

801dd0b

remove logic for handling rebalance callback errors

0a4aa1d

fixes on unit tests, add more unit tests

3f2fe2e

Merge branch 'trunk' of https://github.com/apache/kafka into K8179-p1…

bb98793

…-consumer-protocol

add version varible to subscription and assignment

d1064c4

rebase from p1

829d3c2

more minor fixes

2b2d8b1

update toString

c22266b

Merge branch 'K8179-p1-consumer-protocol' into K8179-p2-client-algorithm

1fa089a

minor fix

16f75ab

found another lurking bug on my code

e049e5f

rebase from trunk

92320ba

new API

ff1b9a6

add version API

908fda4

Merge branch 'trunk' of https://github.com/apache/kafka into K8179-p1…

3e1fb1c

…-consumer-protocol

github comments

2e9a291

Merge branch 'trunk' of https://github.com/apache/kafka into K8179-p1…

bd60fd2

…-consumer-protocol

Merge branch 'trunk' of https://github.com/apache/kafka into K8179-p1…

74a9895

…-consumer-protocol

rebase from trunk

c85ed6e

throw from user rebalance callback

16c13ad

fix unit tests

ec80f42

guozhangwang added 2 commits August 6, 2019 15:47

Merge branch 'trunk' of https://github.com/apache/kafka into K8179-p3…

ef901b0

…-emigrate-callback

github comments

8220d8e

github comments

5ed5a0a

ableegoldman approved these changes Aug 6, 2019

View reviewed changes

hachikuji reviewed Aug 7, 2019

View reviewed changes

github comments

58913e4

adding more debugging

6d07d42

hachikuji reviewed Aug 8, 2019

View reviewed changes

do not lost partitions upon metadata change

fde3c9b

hachikuji approved these changes Aug 8, 2019

View reviewed changes

guozhangwang added 2 commits August 8, 2019 14:15

Merge branch 'trunk' of https://github.com/apache/kafka into K8179-p3…

67b33ab

…-emigrate-callback

github comments

6041a79

guozhangwang merged commit e867a58 into apache:trunk Aug 8, 2019

bbejeck added a commit to bbejeck/kafka that referenced this pull request Aug 16, 2019

For testing Revert "KAFKA-8179: Part 3, Add PartitionsLost API for re…

1bc83b3

…setGenerations and metadata/subscription change (apache#6884)" This reverts commit e867a58.

braedon mentioned this pull request Aug 22, 2019

Consumer proceeds on when ConsumerRebalanceListener fails dpkp/kafka-python#917

Open

guozhangwang mentioned this pull request Nov 1, 2019

KAFKA-9073: check assignment in requestFailed to avoid NPE #7630

Merged

3 tasks

ocadaruma mentioned this pull request Apr 17, 2020

(in progress) Fix duplication on shutdown fixes #23 line/decaton#34

Closed

guozhangwang deleted the K8179-p3-emigrate-callback branch April 24, 2020 23:58

C0urante mentioned this pull request Sep 13, 2022

KAFKA-14220: Update WorkerSinkTask.java #12622

Closed

Conversation

guozhangwang commented Jun 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

guozhangwang commented Aug 6, 2019

Uh oh!

ableegoldman left a comment

Choose a reason for hiding this comment

Uh oh!

hachikuji Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

guozhangwang Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

hachikuji Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

guozhangwang commented Aug 7, 2019

Uh oh!

guozhangwang commented Aug 7, 2019

Uh oh!

guozhangwang commented Aug 7, 2019

Uh oh!

hachikuji Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

guozhangwang commented Aug 8, 2019

Uh oh!

guozhangwang commented Aug 8, 2019

Uh oh!

hachikuji left a comment

Choose a reason for hiding this comment

Uh oh!

hachikuji Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

guozhangwang Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

guozhangwang commented Jun 5, 2019 •

edited

Loading