Skip to content

MINOR: Migrate controllerSocketTimeoutMs from KafkaConfig to AbstractKafkaConfig#22188

Open
yunchipang wants to merge 2 commits intoapache:trunkfrom
yunchipang:migrate-controllerSocketTimeoutMs
Open

MINOR: Migrate controllerSocketTimeoutMs from KafkaConfig to AbstractKafkaConfig#22188
yunchipang wants to merge 2 commits intoapache:trunkfrom
yunchipang:migrate-controllerSocketTimeoutMs

Conversation

@yunchipang
Copy link
Copy Markdown
Contributor

@yunchipang yunchipang commented Apr 30, 2026

  • Migratre controllerSocketTimeoutMs to AbstractKafkaConfig
  • Update usage in NodeToControllerChannelManagerImpl

@github-actions github-actions Bot added triage PRs from the community core Kafka Broker small Small PRs labels Apr 30, 2026
@muralibasani
Copy link
Copy Markdown
Contributor

Would it be better to make another related change https://github.com/apache/kafka/blob/be36babf5567d34de5ec8aaf67de16c5fbc8932d/serv[…]java/org/apache/kafka/server/NodeToControllerRequestThread.java in the same PR ?

…TimeoutMs directly instead of AbstractConfig
Copy link
Copy Markdown
Contributor

@muralibasani muralibasani left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

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.

Thanks, LGTM

@github-actions github-actions Bot removed the triage PRs from the community label May 3, 2026
@@ -98,7 +92,7 @@ void testRetryTimeoutWhileControllerNotAvailable() {
long retryTimeoutMs = 30000;
NodeToControllerRequestThread testRequestThread = new NodeToControllerRequestThread(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just realized this object testRequestThread creation steps are repeated multiple times. Refactoring this to a util method is better ?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants