Skip to content
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

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from

Conversation

ableegoldman
Copy link
Member

@ableegoldman ableegoldman commented Feb 21, 2025

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:

  1. apply the upgrade.from config and perform the 1st rolling bounce from 2.3 or below to a version in [3.5, 3.9]
  2. perform a 2nd rolling bounce to remove the upgrade.from config but without changing the version
  3. perform the 3rd a final rolling bounce to upgrade to 4.0

@ableegoldman ableegoldman changed the title Drop EAGER rebalancing support in Kafka Streams KAFKA-18839: Drop EAGER rebalancing support in Kafka Streams Feb 21, 2025
…nals/StreamsPartitionAssignor.java

Co-authored-by: Matthias J. Sax <[email protected]>
assignorConfiguration.rebalanceProtocol();
} catch (final Exception error) {
throw new AssertionError("Upgrade from " + upgradeFrom + " failed with " + error.getMessage() + "!");
if (upgradeFrom.toString().equals("2.4")) {
Copy link
Member

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 ?

Copy link
Member Author

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

Copy link
Member

@mjsax mjsax Feb 21, 2025

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...)

Copy link
Member Author

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...

@mjsax
Copy link
Member

mjsax commented Feb 21, 2025

\cc @dajac for visibility -- we want to get this into 4.0 release.

@@ -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),
Copy link
Member Author

@ableegoldman ableegoldman Feb 21, 2025

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...?

@dajac dajac added the Blocker This pull request is identified as solving a blocker for a release. label Feb 21, 2025
@ijuma
Copy link
Member

ijuma commented Feb 23, 2025

I think this change means we can also revert all the changes introduced via d1952e8 cc @FrankYang0529

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker This pull request is identified as solving a blocker for a release. streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants