Skip to content

KAFKA-13242: Ensure UpdateFeatures is properly handled in KRaft#11522

Closed
dengziming wants to merge 2 commits intoapache:trunkfrom
dengziming:KAFKA-13242-kraft-update-features
Closed

KAFKA-13242: Ensure UpdateFeatures is properly handled in KRaft#11522
dengziming wants to merge 2 commits intoapache:trunkfrom
dengziming:KAFKA-13242-kraft-update-features

Conversation

@dengziming
Copy link
Copy Markdown
Member

More detailed description of your change
This patch fixes several problems with the UpdateFeatures API in KRaft:

KafkaApis did not properly forward this request type to the controller.
ControllerApis did not handle the request type.
Some other small fixes.

Summary of testing strategy (including rationale)

  1. unit test in FeatureControlManagerTest
  2. I tried to converted UpdateFeaturesTest to use KRaft, but I encountered some problems, for example, we use totally different class in KRaft and zk mode, and we can overwrite data in zk path but we can't overwrite data in KRaft metadata topic, so I decided to handle this in a future PR.

Committer Checklist (excluded from commit message)

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

@dengziming dengziming force-pushed the KAFKA-13242-kraft-update-features branch from 4c7314f to ce0bcd3 Compare November 22, 2021 11:54
@dengziming
Copy link
Copy Markdown
Member Author

Hello @cmccabe ,PTAL, thanks.

Copy link
Copy Markdown
Member

@mumrah mumrah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dengziming, thanks for the patch! I have many similar looking changes locally as part of my KIP-778 prototype. This is a good chunk to split out and get merged ahead of the other changes.

There are some possible changes to KIP-584 coming from that discussion, so let's see what happens there. I'll try to take a look at this next week (after thanksgiving).

@mumrah mumrah self-assigned this Nov 23, 2021
@mumrah mumrah added the kraft label Nov 23, 2021
@dengziming dengziming force-pushed the KAFKA-13242-kraft-update-features branch from ce0bcd3 to 3fe8be2 Compare December 15, 2021 15:35
@mumrah
Copy link
Copy Markdown
Member

mumrah commented Dec 15, 2021

@dengziming, we have a few changes to the feature flags coming in KIP-778. Let's hold off on merging this until the KIP is adopted.

@dengziming
Copy link
Copy Markdown
Member Author

Thank you @mumrah , I am checking KIP-778 about this.

@dengziming
Copy link
Copy Markdown
Member Author

See #12036

@dengziming dengziming closed this Apr 15, 2022
@dengziming dengziming deleted the KAFKA-13242-kraft-update-features branch November 24, 2022 06:47
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.

2 participants