Copilot commented on code in PR #23687:
URL: https://github.com/apache/kafka/pull/23687#discussion_r4176522899
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractHeartbeatRequestManager.java:
##########
@@ -114,7 +114,8 @@ public abstract class AbstractHeartbeatRequestManager<R
extends AbstractResponse
this.maxPollIntervalMs =
config.getInt(CommonClientConfigs.MAX_POLL_INTERVAL_MS_CONFIG);
long retryBackoffMs =
config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG);
long retryBackoffMaxMs =
config.getLong(ConsumerConfig.RETRY_BACKOFF_MAX_MS_CONFIG);
- this.heartbeatRequestState = new HeartbeatRequestState(logContext,
time, 0, retryBackoffMs,
+ int requestTimeoutMs =
config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG);
Review Comment:
`request.timeout.ms` accepts `0` (`ConsumerConfig` validates it with
`atLeast(0)`), so this can still initialize the heartbeat timer to zero. While
the first heartbeat is in flight, both `poll()` and `maximumTimeToWait()` can
then keep returning zero until the network request is timed out, preserving the
startup spin for a valid configuration. Clamp this internal placeholder
interval to at least 1 ms and add a zero-timeout case to the regression test.
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/StreamsGroupHeartbeatRequestManager.java:
##########
@@ -413,10 +413,11 @@ public StreamsGroupHeartbeatRequestManager(final
LogContext logContext,
long retryBackoffMs =
config.getLong(ConsumerConfig.RETRY_BACKOFF_MS_CONFIG);
long retryBackoffMaxMs =
config.getLong(ConsumerConfig.RETRY_BACKOFF_MAX_MS_CONFIG);
this.heartbeatState = new HeartbeatState(streamsRebalanceData,
membershipManager, maxPollIntervalMs, time);
+ int requestTimeout =
config.getInt(ConsumerConfig.REQUEST_TIMEOUT_MS_CONFIG);
Review Comment:
`request.timeout.ms` accepts `0` (`ConsumerConfig` validates it with
`atLeast(0)`), so Streams can still start with a zero heartbeat timer. While
the initial heartbeat is in flight, subsequent polls may therefore return zero
until that request times out, leaving the valid zero-timeout configuration
exposed to the busy-spin this change is intended to remove. Clamp the
placeholder interval to at least 1 ms and cover this configuration in the
regression test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
