-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KAFKA-18839: Drop EAGER rebalancing support in Kafka Streams #18988
base: trunk
Are you sure you want to change the base?
KAFKA-18839: Drop EAGER rebalancing support in Kafka Streams #18988
Conversation
streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
Outdated
Show resolved
Hide resolved
tests/kafkatest/tests/streams/streams_cooperative_rebalance_upgrade_test.py
Outdated
Show resolved
Hide resolved
...ams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java
Outdated
Show resolved
Hide resolved
…nals/StreamsPartitionAssignor.java Co-authored-by: Matthias J. Sax <mjsax@apache.org>
...main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java
Outdated
Show resolved
Hide resolved
...main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java
Outdated
Show resolved
Hide resolved
streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Matthias J. Sax <mjsax@apache.org>
...main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java
Show resolved
Hide resolved
assignorConfiguration.rebalanceProtocol(); | ||
} catch (final Exception error) { | ||
throw new AssertionError("Upgrade from " + upgradeFrom + " failed with " + error.getMessage() + "!"); | ||
if (upgradeFrom.toString().equals("2.4")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems we would need to also check 2.5, ... 3.9, not just 2.4 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do, it loops over all the versions starting from the incompatible upgrades, we don't only check 2.4 it's just that 2.4 is where we switch from asserting that an exception is thrown to asserting that it can be instantiated without throwing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we loop, but when upgradeFrom
changes, should the condition not be:
if (upgradeFrom.toString().equals("2.4")
|| upgradeFrom.toString().equals("2.5")
...
|| upgradeFrom.toString().equals("3.9")
// need to add a new version here for every new release
) {
Otherwise we set beforeCooperative = false
only for a single loop iteration when upgradeFrom = 2.4
?
What actually makes me suggest to flip from logic and initialize beforeCooperative = false
, and set it to true
for versions 0.10.0 to 2.3 to avoid that we need to keep updating this test for very future release (what we would most likely forget...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope because beforeCooperative
is declared outside the loop so once it gets updated it stays on the new value..? am I going crazy here?
I mean the testing is passing so...
.../java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfigurationTest.java
Outdated
Show resolved
Hide resolved
.../java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfigurationTest.java
Outdated
Show resolved
Hide resolved
\cc @dajac for visibility -- we want to get this into |
streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
Outdated
Show resolved
Hide resolved
…g-bounce upgrade test
@@ -33,8 +33,7 @@ | |||
str(LATEST_3_3), str(LATEST_3_4), str(LATEST_3_5), str(LATEST_3_6), | |||
str(LATEST_3_7), str(LATEST_3_8), str(LATEST_3_9), str(DEV_BRANCH)] | |||
|
|||
metadata_2_versions = [str(LATEST_0_11), str(LATEST_1_0), str(LATEST_1_1), str(LATEST_2_0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a little confused about why 2.1 - 2.3 were not included in this matrix...?
In 3.1 we deprecated the eager rebalancing protocol and marked it for removal in a later release. We aim to officially drop support and remove the protocol from Streams in 4.0. Note that we are only removing the protocol itself from Streams for now, and will realize the actual benefit from dropping EAGER by cleaning up the task assignment & lifecycle management in a future PR.
The effect of this PR is that it will no longer be possible to perform a live upgrade Kafka Streams directly to 4.0 from version 2.3 or below. Users will have to go through a bridge release between 2.4 - 3.9 instead.
Note that several other incompatibilities that depend on the
upgrade.from
config still remain, which require it to be used when upgrading from [2.4, 3.4] to 3.5 or above. For this reason we recommend using a version in [3.5, 3.9] as the bridge release, so that only 3 rolling bounces are needed to upgrade. The recommended path is as follows:upgrade.from
config and perform the 1st rolling bounce from 2.3 or below to a version in [3.5, 3.9]upgrade.from
config but without changing the version