KAFKA-9821: persist followup rebalance in assignment and consolidate rebalance triggering mechanisms by ableegoldman · Pull Request #8596 · apache/kafka

ableegoldman · 2020-05-01T01:33:14Z

This patch fixes KAFKA-9821 for Streams by persisting the required rebalance in the assignment. If a member fails and restarts it will receive its last assignment and notify the thread to trigger a new rebalance, even with static membership.

Note: in the consumer client, the member who has to revoke some partitions is responsible for triggering the followup rebalance. Here we are actually encoding the required rebalance for the member who was supposed to receive these partitions. Since this only signals to the client to request a rebalance upon the next poll in both cases, and the rebalance schedule is reset in onAssignment, this should not result in extra (unnecessary) rebalances even if one member lags the other in requesting this.

This patch also consolidates the two rebalancing triggering mechanisms currently used by the Streams partition assignor: the AssignorError and the newly introduced nextScheduleRebalanceMs. Since the latter encompasses the former, we can remove the REBALANCE_NEEDED assignor error and just schedule the next rebalance for 0L ms to trigger it immediately. This has the nice side effect of removing the only recoverable AssignorError, so we no longer have to worry about handling this error and can just assume it to be fatal and shut down.

ableegoldman · 2020-05-01T01:33:35Z

@guozhangwang @vvcephei @cadonna

ableegoldman · 2020-05-01T02:29:43Z

Kicked off the version probing system test since I'm mucking around in the rebalance triggering code and log messages, and I've broken it that way before 😅 : https://jenkins.confluent.io/job/system-test-kafka-branch-builder/3926/

edit: yup, broke the version probing test. Investigation ongoing

ableegoldman · 2020-05-02T00:23:04Z

Also cc @mjsax for some feedback on making the rebalancing comprehensible in system test logs

ableegoldman · 2020-05-02T00:32:42Z

We lose information here, but now log the reason for the scheduled rebalance in a (hopefully) more clear way during onAssignment

ableegoldman · 2020-05-02T00:35:58Z

Previously we were doing this logging at the client-level, meaning we could end up in the confusing situation of logging Stable rebalance.. for one client and Unstable rebalance... for another, all in the same rebalance. Now we only log the final result after all client assignments are encoded, and log which clients actually are scheduling said rebalance at the debug level.

ableegoldman · 2020-05-02T00:38:28Z

This encodes an immediate followup rebalance for the consumer who would be receiving the revoked tasks. The consumer that is doing the revoking will also request a followup rebalance (in the client code) but due to KAFKA-9821 this cannot be relied upon when static membership is used.

ableegoldman · 2020-05-02T00:38:58Z

This is now the only usage of the assignment error code

ableegoldman · 2020-05-02T00:41:07Z

Here we check for and log all the possible reasons for a followup rebalance to be scheduled on this consumer. It should be clear whether (and why) a rebalance is required by looking at the logs of either the group leader or the member(s) requesting the rebalance.

mjsax · 2020-05-02T00:41:22Z

Retest this please.

ableegoldman · 2020-05-02T02:01:11Z

Version probing test passing locally now, kicked off a branch builder to confirm: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/3927/

ableegoldman · 2020-05-02T03:21:12Z

System test run passed (as did all three builds, wow!)

vvcephei

LGTM (just the one minor comment about the config key, above)

Waiting for @mjsax to take a look also.

Thanks, @ableegoldman !

ableegoldman · 2020-05-08T19:05:38Z

Kicking off the version probing system test again after rebasing on top of Bruno's changes/fix (because I can't run it locally again... 😕 ) -- https://jenkins.confluent.io/job/system-test-kafka-branch-builder/3934/

vvcephei · 2020-05-08T20:38:08Z

test this please

vvcephei · 2020-05-08T20:38:15Z

test this please

ableegoldman · 2020-05-08T21:03:52Z

Messed up the system test logs during the rebase, kicked off a new version probing test: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/3935

edit: system test passed

vvcephei · 2020-05-08T21:18:43Z

test this please

vvcephei · 2020-05-08T21:18:50Z

test this please

vvcephei · 2020-05-08T21:18:58Z

test this please

vvcephei · 2020-05-08T23:36:09Z

Hey @ableegoldman , there were a bunch of failures:
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[false]
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[false]
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]

org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[false]
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin

org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[false]
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[false]
org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin

Do you think those are related?

vvcephei · 2020-05-08T23:36:52Z

System test results, for posterity: http://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2020-05-08--001.1588974111--ableegoldman--MINOR-schedule-immediate-rebalance-on-revoked--2ff153d/report.html

ableegoldman · 2020-05-08T23:40:07Z

Yeah, I just saw...honestly I'm not sure what to conclude here since all of them did pass at least once, and all of the failed tests are known to be flaky. But it does seem to be a higher-than-normal failure rate so I'm investigating

ableegoldman · 2020-05-09T00:08:02Z

@vvcephei Yeah I can locally reproduce the EOSBetaUpgradeTest failures pretty reliably even on trunk, I don't think it's related to this PR. Not sure there's anything to do until we have the assignment hook for integration tests, except possibly mark it @ignore...

Not sure what's up with GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin, I can't reproduce that one locally but it's pretty flaky as well according to the ticket

ableegoldman · 2020-05-09T00:14:02Z

Oh hm actually I can't seem to reproduce it on trunk anymore, at least not as frequently as on this branch. Maybe we should block merging this PR until we have the test hook in place

vvcephei · 2020-05-11T20:29:54Z

retest this please

vvcephei · 2020-05-11T20:30:06Z

No harm in trying again :)

vvcephei · 2020-05-12T17:45:08Z

test this please

vvcephei · 2020-05-12T17:45:17Z

test this please

vvcephei · 2020-05-12T17:45:28Z

retest this please

* 'trunk' of github.com:apache/kafka: MINOR: add option to rebuild source for system tests (apache#6656) KAFKA-9850 Move KStream#repartition operator validation during Topolo… (apache#8550) MINOR: Add a duplicate() method to Message classes (apache#8556) KAFKA-9966: add internal assignment listener to stabilize eos-beta upgrade test (apache#8648) MINOR: Replace null with an actual value for timestamp field in InsertField SMT unit tests (apache#8649) MINOR: Fix ProcessorContext JavaDocs and stream-time computation (apache#8603) MINOR: improve tests for TopologyTestDriver (apache#8631) KAFKA-9821: consolidate Streams rebalance triggering mechanisms (apache#8596) KAFKA-9669; Loosen validation of inner offsets for older message formats (apache#8647) KAFKA-8770: KIP-557: Drop idempotent KTable source updates (apache#8254) MINOR: Remove allow concurrent test (apache#8641)

…he#8596) Persist followup rebalance in assignment and consolidate rebalance triggering mechanisms Reviewers: John Roesler <vvcephei@apache.org>

ableegoldman commented May 2, 2020

View reviewed changes

Comment thread streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java Outdated

ableegoldman commented May 2, 2020

View reviewed changes

mjsax added the streams label May 2, 2020

ableegoldman commented May 2, 2020

View reviewed changes

vvcephei approved these changes May 4, 2020

View reviewed changes

ableegoldman force-pushed the MINOR-schedule-immediate-rebalance-on-revoked branch from 24e2f77 to 39ea82c Compare May 5, 2020 18:31

ableegoldman added 4 commits May 8, 2020 10:47

consolidate rebalance triggering mechanism, workaround for KAFKA-9821

306184c

consolidate followup rebalance reason log messages

615ed0a

further improve logging

ae6c1c9

bump stabilized log to info

eb206b8

ableegoldman force-pushed the MINOR-schedule-immediate-rebalance-on-revoked branch from 39ea82c to eb206b8 Compare May 8, 2020 17:48

fix log message

2ff153d

temporarily Ignore upgrade test

0966ea5

ableegoldman mentioned this pull request May 12, 2020

KAFKA-9966: add internal assignment listener to stabilize eos-beta upgrade test #8648

Merged

vvcephei merged commit 58f7a97 into apache:trunk May 12, 2020

ableegoldman deleted the MINOR-schedule-immediate-rebalance-on-revoked branch May 12, 2020 23:01

Uh oh!

Conversation

ableegoldman commented May 1, 2020

Uh oh!

ableegoldman commented May 1, 2020

Uh oh!

ableegoldman commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ableegoldman commented May 2, 2020

Uh oh!

Uh oh!

ableegoldman May 2, 2020

Choose a reason for hiding this comment

Uh oh!

ableegoldman May 2, 2020

Choose a reason for hiding this comment

Uh oh!

ableegoldman May 2, 2020

Choose a reason for hiding this comment

Uh oh!

ableegoldman May 2, 2020

Choose a reason for hiding this comment

Uh oh!

ableegoldman May 2, 2020

Choose a reason for hiding this comment

Uh oh!

mjsax commented May 2, 2020

Uh oh!

ableegoldman commented May 2, 2020

Uh oh!

ableegoldman commented May 2, 2020

Uh oh!

vvcephei left a comment

Choose a reason for hiding this comment

Uh oh!

ableegoldman commented May 8, 2020

Uh oh!

vvcephei commented May 8, 2020

Uh oh!

vvcephei commented May 8, 2020

Uh oh!

ableegoldman commented May 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vvcephei commented May 8, 2020

Uh oh!

vvcephei commented May 8, 2020

Uh oh!

vvcephei commented May 8, 2020

Uh oh!

vvcephei commented May 8, 2020

Uh oh!

vvcephei commented May 8, 2020

Uh oh!

ableegoldman commented May 8, 2020

Uh oh!

ableegoldman commented May 9, 2020

Uh oh!

ableegoldman commented May 9, 2020

Uh oh!

vvcephei commented May 11, 2020

Uh oh!

vvcephei commented May 11, 2020

Uh oh!

vvcephei commented May 12, 2020

Uh oh!

vvcephei commented May 12, 2020

Uh oh!

vvcephei commented May 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ableegoldman commented May 1, 2020 •

edited

Loading

ableegoldman commented May 8, 2020 •

edited

Loading