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-18736: Add pollOnClose() and maximumTimeToWait() #19233

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

cadonna
Copy link
Member

@cadonna cadonna commented Mar 18, 2025

Adds pollOnClose() and maximumTimeToWait() to the Streams
group heartbeat request manager.

Reviewers: Lucas Brutschy [email protected]

@cadonna cadonna requested a review from lucasbru March 18, 2025 20:06
@cadonna cadonna added streams KIP-1071 PRs related to KIP-1071 consumer and removed consumer labels Mar 18, 2025
@cadonna cadonna requested review from bbejeck and mjsax March 18, 2025 20:08
Copy link
Member

@lucasbru lucasbru 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 PR, @cadonna ! I left minor comment, otherwise LGTM

@Override
public long maximumTimeToWait(long currentTimeMs) {
pollTimer.update(currentTimeMs);
if (pollTimer.isExpired() || (membershipManager.shouldNotWaitForHeartbeatInterval() && !heartbeatRequestState.requestInFlight())) {
Copy link
Member

Choose a reason for hiding this comment

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

unnecessary parantheses

@cadonna cadonna force-pushed the streams_group_heartbeat_request_manager-6 branch 2 times, most recently from 74283e1 to 4f43a67 Compare March 24, 2025 15:59
cadonna added 2 commits March 24, 2025 21:28
Adds pollOnClose() and maximumTimeToWait() to the Streams
group heartbeat request manager.
@cadonna cadonna force-pushed the streams_group_heartbeat_request_manager-6 branch from 4f43a67 to 9616493 Compare March 24, 2025 20:31
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.

None yet

2 participants