cmccabe commented on code in PR #19745:
URL: https://github.com/apache/kafka/pull/19745#discussion_r2098839394
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -411,7 +411,7 @@ class BrokerServer(
config,
"heartbeat",
s"broker-${config.nodeId}-",
- config.brokerSessionTimeoutMs / 2 // KAFKA-14392
+ config.brokerHeartbeatIntervalMs
Review Comment:
> Given the broker side request timeout and scheduling logic this period is
actually 2 * brokerHeartbeatIntervalMs. Are you planning to fix this so that
the period is actually brokerHeartbeatIntervalMs?
If I understand the question right, you're asking whether we should unify
the request timeout and the resend timeout? I do think we should do that, but
since it's a more complex change, I want to do it in a follow-up PR.
> Should we change the config validation logic so that 2 *
brokerHeartbeatIntervalMs < brokerSessionTimeoutMs?
It's a bit tricky to add validations like that since it may cause currently
working setups to not start up any more. I do think we should add a WARN log or
something if the heartbeat configuration doesn't make sense. I think that also
belongs in a follow-on PR.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
