KAFKA-19457: Make share group init retry interval configurable. #20104
Conversation
github-actions
bot
added
triage
smjn
added
KIP-932
| @@ -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, | |||
There was a problem hiding this 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.
|
@AndrewJSchofield Thanks for the comments, incorporated. |
| @@ -437,6 +445,8 @@ public GroupCoordinatorConfig(AbstractConfig config) { | |||
| this.shareGroupMaxHeartbeatIntervalMs = config.getInt(GroupCoordinatorConfig.SHARE_GROUP_MAX_HEARTBEAT_INTERVAL_MS_CONFIG); | |||
| this.shareGroupMaxSize = config.getInt(GroupCoordinatorConfig.SHARE_GROUP_MAX_SIZE_CONFIG); | |||
| this.shareGroupAssignors = shareGroupAssignors(config); | |||
| int initializeRetryMs = config.getInt(GroupCoordinatorConfig.SHARE_GROUP_INITIALIZE_RETRY_INTERVAL_MS_CONFIG); | |||
| this.shareGroupInitializeRetryIntervalMs = Math.max(initializeRetryMs, this.offsetCommitTimeoutMs); | |||
There was a problem hiding this comment.
Should this comparison add to SHARE_GROUP_INITIALIZE_RETRY_INTERVAL_MS_DOC for clarification ?
|
@smjn The code change looks good to me. Please could you triage the failed tests and make sure they are appropriately tracked. Thanks. |
|
Unrelated Flaky/failed tests: FAILED ❌ LogRecoveryTest > testHWCheckpointWithFailuresMultipleLogSegments() - already tracked in https://issues.apache.org/jira/browse/KAFKA-19452 FLAKY |
GroupMetadataManager.shareGroupHeartbeat, we check for topics ininitializingstate and if they are a certain amount of time old, weissue retry requests for the same.
offsetsCommitTimeoutMsand was not configurable.supply the value. The default is
30_000which is aheuristic based on the fact that the share coordinator
persisterretries request with exponential backoff, with upper cap of
30_000seconds.
Reviewers: Apoorv Mittal [email protected], Lan Ding
[email protected], TaiJuWu [email protected], Andrew Schofield
[email protected]