Skip to content

KAFKA-19432: Add an ERROR log message if broker.heartbeat.interval.ms is too large#20046

Merged
chia7712 merged 9 commits into
apache:trunkfrom
FrankYang0529:KAFKA-19432
Sep 3, 2025
Merged

KAFKA-19432: Add an ERROR log message if broker.heartbeat.interval.ms is too large#20046
chia7712 merged 9 commits into
apache:trunkfrom
FrankYang0529:KAFKA-19432

Conversation

@FrankYang0529

@FrankYang0529 FrankYang0529 commented Jun 26, 2025

Copy link
Copy Markdown
Member
  • Log error message if broker.heartbeat.interval.ms * 2 is large than
    broker.session.timeout.ms.
  • Add test case
    testLogBrokerHeartbeatIntervalMsShouldBeLowerThanHalfOfBrokerSessionTimeoutMs.

Reviewers: Chia-Ping Tsai chia7712@gmail.com

… is too large

Signed-off-by: PoAn Yang <payang@apache.org>
@github-actions github-actions Bot added core Kafka Broker small Small PRs labels Jun 26, 2025
@FrankYang0529
FrankYang0529 requested a review from chia7712 June 28, 2025 06:30
" to prevent frequent changes in ISR")

if (brokerHeartbeatIntervalMs * 2 > brokerSessionTimeoutMs) {
error(s"${KRaftConfigs.BROKER_HEARTBEAT_INTERVAL_MS_CONFIG} ($brokerHeartbeatIntervalMs ms) must be less than or equal to half of the ${KRaftConfigs.BROKER_SESSION_TIMEOUT_MS_CONFIG} ($brokerSessionTimeoutMs ms). " +

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.

The message says "must be less than or equal to half", but the server does not actually stop. Perhaps the description could be phrased more clearly?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@chia7712 do you think that it's better to throw error if broker.heartbeat.interval.ms is larger than half of broker.session.timeout.ms? Or we can change the error message like following:

      error(s"${KRaftConfigs.BROKER_HEARTBEAT_INTERVAL_MS_CONFIG} ($brokerHeartbeatIntervalMs ms) is larger than half of the ${KRaftConfigs.BROKER_SESSION_TIMEOUT_MS_CONFIG} ($brokerSessionTimeoutMs ms). " +
        "If missing anyone heartbeat request, the broker loses broker lease. " +
        s"Please increase ${KRaftConfigs.BROKER_SESSION_TIMEOUT_MS_CONFIG} or decrease ${KRaftConfigs.BROKER_HEARTBEAT_INTERVAL_MS_CONFIG}.")

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.

Yes, it would be better to fail fast. However, since session timeout is used by the quorum controller, the broker itself is not aware of the actual session timeout configured for the quorum controller. Perhaps we could improve the error message to make this distinction cleaer, incorporating the above explanation

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.

@FrankYang0529 any updates? it would be useful to remind users that session timeout is controlled by quorum controller rather than broker

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sorry for late. Update it.

Signed-off-by: PoAn Yang <payang@apache.org>
Signed-off-by: PoAn Yang <payang@apache.org>
Signed-off-by: PoAn Yang <payang@apache.org>
@FrankYang0529
FrankYang0529 requested a review from chia7712 August 28, 2025 04:59

@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.

LGTM

@chia7712
chia7712 merged commit ea5b5fe into apache:trunk Sep 3, 2025
23 checks passed
@FrankYang0529
FrankYang0529 deleted the KAFKA-19432 branch September 5, 2025 02:56
eduwercamacaro pushed a commit to littlehorse-enterprises/kafka that referenced this pull request Nov 12, 2025
…is too large (apache#20046)

* Log error message if `broker.heartbeat.interval.ms * 2` is large than
`broker.session.timeout.ms`.
* Add test case

`testLogBrokerHeartbeatIntervalMsShouldBeLowerThanHalfOfBrokerSessionTimeoutMs`.

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Kafka Broker small Small PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants