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-17584: Fix incorrect synonym handling for dynamic log configurations #17696

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

clolov
Copy link
Contributor

@clolov clolov commented Nov 5, 2024

This is a cherry-pick of #17258 to 3.7.2

This commit differs from the original by using the old (read 3.7) references to the configurations and not changing as many unit tests

@clolov
Copy link
Contributor Author

clolov commented Nov 5, 2024

Heya @cmccabe and @showuon, in order to add this to the 3.7.2 release I have made an attempt to port the changes. Would you be willing to review this?

+cc @mjsax for visibility

Copy link
Contributor

@divijvaidya divijvaidya left a comment

Choose a reason for hiding this comment

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

I compared this with the backport in https://github.com/apache/kafka/pull/17278/files and the changes look good.

Let's give some time for original authors to take a look before we merge this (also CI tests are failing).

@divijvaidya divijvaidya added the core Kafka Broker label Nov 6, 2024
@mjsax
Copy link
Member

mjsax commented Nov 13, 2024

@clolov @cmccabe @showuon -- Can we make progress on this cherry-pick? It's currently blocking me from creating the first RC.

Copy link
Contributor

@cmccabe cmccabe left a comment

Choose a reason for hiding this comment

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

LGTM after looking at this side-by-side with the original change

Thanks @clolov

@mjsax
Copy link
Member

mjsax commented Nov 18, 2024

Triggered a new CI run -- can merge this afterwards.

@mjsax
Copy link
Member

mjsax commented Nov 19, 2024

@clolov It seems ReplicaManagerTest.testRemoteFetchExpiresPerSecMetric fails all the time. Thoughts? No idea if related or not.

@clolov
Copy link
Contributor Author

clolov commented Nov 19, 2024

Heya @mjsax. The test passes locally when I run it individually, as part of the class and as part of the whole test suite. It also appears to have been passing as build 2 of the same PR. If that is the only concern is there a way we can get a complete new clean build which doesn't reuse any old build artifacts just to see whether it is persistent?

@mjsax mjsax merged commit 1c4b02a into apache:3.7 Nov 25, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Kafka Broker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants