Skip to content

KAFKA-20970 Another busy loop happens if auto.commit.interval.ms is less than bootstrap.resolve.timeout.ms - #23227

Open
m1a2st wants to merge 2 commits into
apache:trunkfrom
m1a2st:KAFKA-20970
Open

KAFKA-20970 Another busy loop happens if auto.commit.interval.ms is less than bootstrap.resolve.timeout.ms#23227
m1a2st wants to merge 2 commits into
apache:trunkfrom
m1a2st:KAFKA-20970

Conversation

@m1a2st

@m1a2st m1a2st commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

CommitRequestManager#maximumTimeToWait() returned the raw remaining
time of the auto-commit timer, which becomes 0 once the interval
elapses. However, poll() cannot actually send an auto-commit until
the coordinator is known; it returns EMPTY while
coordinatorRequestManager.coordinator() is empty. When bootstrap DNS
resolution is still in progress (bootstrap.resolve.timeout.ms > 0) and
auto.commit.interval.ms is shorter than the resolution window, the
auto-commit timer can expire before the coordinator is discovered.
remainingMs() then continues to return 0 indefinitely, since the
timer is not reset until the application thread successfully sends an
auto-commit.

This 0 is propagated to
ConsumerNetworkThread#cachedMaximumTimeToWait, which bounds the
pollTimeout in AsyncKafkaConsumer#pollForFetches() to 0, causing
the application and network threads to busy-loop for as long as the
coordinator remains unknown.

Reviewers: Chia-Ping Tsai chia7712@gmail.com, Andrew Schofield
aschofield@confluent.io

@github-actions github-actions Bot added triage PRs from the community consumer clients small Small PRs labels Aug 21, 2026

@chia7712 chia7712 left a comment

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.

@m1a2st Would you mind enabling DNS resolution in this patch? It can help us catch unexpected latency or resource issues

@AndrewJSchofield

Copy link
Copy Markdown
Member

Might StreamsGroupHeartbeatRequestManager needs a similar check for coordinatorRequestManager.coordinator().isEmpty()? It seems like an outlier.

@AndrewJSchofield AndrewJSchofield left a comment

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.

Thanks for the PR. Looks good to me. Just one suggestion for the test.

@Test
public void testMaximumTimeToWaitWhenCoordinatorUnknownDoesNotSpin() {
CommitRequestManager commitRequestManager = create(true, 100);

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.

I wonder whether when(coordinatorRequestManager.coordinator()).thenReturn(Optional.empty()); might be preferable here. Makes it clear what is being asserted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed it.

@github-actions github-actions Bot removed the small Small PRs label Aug 22, 2026
@m1a2st

m1a2st commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

Might StreamsGroupHeartbeatRequestManager needs a similar check for coordinatorRequestManager.coordinator().isEmpty()? It seems like an outlier.

Good catch. I'll address this in this patch as well.

@github-actions github-actions Bot removed the triage PRs from the community label Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants