Skip to content

KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined#21918

Open
kevin-wu24 wants to merge 4 commits intoapache:trunkfrom
kevin-wu24:KAFKA-20380
Open

KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined#21918
kevin-wu24 wants to merge 4 commits intoapache:trunkfrom
kevin-wu24:KAFKA-20380

Conversation

@kevin-wu24
Copy link
Copy Markdown
Contributor

@kevin-wu24 kevin-wu24 commented Mar 31, 2026

What Changed

  • In KafkaConfig#effectiveAdvertisedControllerListeners, when
    advertised.listeners is not defined but controller.quorum.voters is,
    we should advertise the controller.quorum.voters endpoint.

Testing

Unit tests in KafkaConfigTest.

…er.quorum.voters, which is expected to be routable
@github-actions github-actions bot added triage PRs from the community kraft small Small PRs and removed triage PRs from the community labels Mar 31, 2026
Copy link
Copy Markdown
Member

@jsancio jsancio left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @kevin-wu24 . Quick review to provide feedback on the strategy used to solve this problem.

// should be determined from the static voter set since the local listeners may
// not be a routable address for other voters in the cluster.
Endpoints leaderEndpoints = partitionState.lastKraftVersion().isReconfigSupported()
? localListeners
Copy link
Copy Markdown
Member

@jsancio jsancio Mar 31, 2026

Choose a reason for hiding this comment

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

Did you consider fixing this by making sure that the localListener passed to KafkaRaftClient is always correct?

Copy link
Copy Markdown
Contributor Author

@kevin-wu24 kevin-wu24 Mar 31, 2026

Choose a reason for hiding this comment

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

I did consider that approach, but my main question was:

How does the caller of the KafkaRaftClient constructor, KafkaRaftManager, know what its current kraft version is without reading the entire log prior to constructing the client? The KRaftControlRecordStateMachine already does this read within KRaft.

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.

Local endpoint is computed using the configuration. The resolution logic could be in this order:

  1. Use the advertise listener if it exists
  2. Use the controller.quorum.voters if it exists
  3. Do a reverse lookup of the listener.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see. This issue is technically independent of kraft.version. Let me update the JIRA to reflect that.

@kevin-wu24 kevin-wu24 changed the title KAFKA-20380: kraft.version=0 leaders should advertise their endpoint from controller.quorum.voters KAFKA-20380: controller.quorum.voters should act as advertised.listeners if the latter is not defined Mar 31, 2026
…controller.quorum.voters, which is expected to be routable"

This reverts commit 14bd70c.
@github-actions github-actions bot added the core Kafka Broker label Apr 1, 2026
@kevin-wu24
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @jsancio. I have pushed some changes to make sure the localListeners passed to the KafkaRaftClient is correct, PTAL.

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