AndrewJSchofield commented on code in PR #20104:
URL: https://github.com/apache/kafka/pull/20104#discussion_r2185151611
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorConfig.java:
##########
@@ -484,6 +493,9 @@ public GroupCoordinatorConfig(AbstractConfig config) {
require(shareGroupSessionTimeoutMs <= shareGroupMaxSessionTimeoutMs,
String.format("%s must be less than or equal to %s",
SHARE_GROUP_SESSION_TIMEOUT_MS_CONFIG,
SHARE_GROUP_MAX_SESSION_TIMEOUT_MS_CONFIG));
+ require(shareGroupInitializeRetryIntervalMs >= offsetCommitTimeoutMs,
Review Comment:
I think this needs to be changed a little. The problem is that you've added
a new internal (and thus undocumented) configuration to allow configuration of
something which should in principle not need tweaking. That's fine. However,
the validation of the configs can now break if someone changes a documented
configuration, and they will not know about the internal configuration. It
would be better to ensure in the code that the
`shareGroupInitializeRetryIntervalMs` is not smaller than the
`offsetCommitTimeoutMs` just using assignment to an appropriate value.
--
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
