Skip to content

KAFKA-18499 Clean up zookeeper from LogConfig#18583

Merged
chia7712 merged 4 commits intoapache:trunkfrom
mingdaoy:KAFKA-18499
Jan 25, 2025
Merged

KAFKA-18499 Clean up zookeeper from LogConfig#18583
chia7712 merged 4 commits intoapache:trunkfrom
mingdaoy:KAFKA-18499

Conversation

@mingdaoy
Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/KAFKA-18499

Committer Checklist (excluded from commit message)

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

@github-actions github-actions Bot added triage PRs from the community core Kafka Broker small Small PRs labels Jan 16, 2025
Copy link
Copy Markdown
Collaborator

@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

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

Hello @mingdaoy, Thanks for this PR. Please also cleanup the LogManager.scala, there are some comments and code still contained zookeeper.

@m1a2st
Copy link
Copy Markdown
Collaborator

m1a2st commented Jan 17, 2025

https://issues.apache.org/jira/browse/KAFKA-18544 also can handle in this PR, WDYT?

@github-actions github-actions Bot added storage Pull requests that target the storage module and removed triage PRs from the community labels Jan 17, 2025
@mingdaoy mingdaoy changed the title KAFKA-18499 Clean up zookeeper in ConfigHandler KAFKA-18499 Clean up zookeeper in ConfigHandler, LogConfig Jan 17, 2025
@github-actions github-actions Bot removed the small Small PRs label Jan 17, 2025
@mingdaoy
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot added the small Small PRs label Jan 21, 2025
@mingdaoy mingdaoy force-pushed the KAFKA-18499 branch 2 times, most recently from d0f6877 to f6eafba Compare January 23, 2025 20:12
class BrokerConfigHandler(private val brokerConfig: KafkaConfig,
private val quotaManagers: QuotaManagers) extends ConfigHandler with Logging {
def processConfigChanges(brokerId: String, properties: Properties): Unit = {
if (brokerId == ZooKeeperInternals.DEFAULT_STRING)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please revert those change as this is unrelated to this PR

@chia7712 chia7712 changed the title KAFKA-18499 Clean up zookeeper in ConfigHandler, LogConfig KAFKA-18499 Clean up zookeeper from LogConfig Jan 24, 2025
@chia7712
Copy link
Copy Markdown
Member

the ConfigHandler has been addressed by #18617

@mingdaoy
Copy link
Copy Markdown
Contributor Author

mingdaoy commented Jan 25, 2025

I ran the tests locally and passed

Java 23 summary

testVerifyFetchAndAcknowledgeSync()
image

testReplicaManagerFetchShouldProceed()
image

testDelayedInitializationShouldCompleteFetchRequest()
image

testNoMessagesSentExceptionFromOverlappingPatterns()
image

ClientIdQuotaTest
image

@chia7712 chia7712 merged commit c23d4a0 into apache:trunk Jan 25, 2025
@mingyen066 mingyen066 mentioned this pull request Jan 25, 2025
3 tasks
Comment thread core/src/test/scala/unit/kafka/log/LogConfigTest.scala
pranavt84 pushed a commit to pranavt84/kafka that referenced this pull request Jan 27, 2025
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
airlock-confluentinc Bot pushed a commit to confluentinc/kafka that referenced this pull request Jan 27, 2025
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
chia7712 pushed a commit that referenced this pull request Feb 6, 2025
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
pdruley pushed a commit to pdruley/kafka that referenced this pull request Feb 12, 2025
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
manoj-mathivanan pushed a commit to manoj-mathivanan/kafka that referenced this pull request Feb 19, 2025
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved core Kafka Broker small Small PRs storage Pull requests that target the storage module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants