dajac commented on code in PR #16458:
URL: https://github.com/apache/kafka/pull/16458#discussion_r1663919753
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorConfig.java:
##########
@@ -297,61 +333,92 @@ public class GroupCoordinatorConfig {
* Also, when a topic is deleted via the delete-topic request, upon
propagated metadata update any group's
* committed offsets for that topic will also be deleted without extra
retention period.
*/
- public final long offsetsRetentionMs;
+ public long offsetsRetentionMs() {
+ return
config.getInt(GroupCoordinatorConfig.OFFSETS_RETENTION_MINUTES_CONFIG) * 60L *
1000L;
+ }
/**
* Offset commit will be delayed until all replicas for the offsets topic
receive the commit
* or this timeout is reached
*/
- public final int offsetCommitTimeoutMs;
+ public int offsetCommitTimeoutMs() {
+ return
config.getInt(GroupCoordinatorConfig.OFFSET_COMMIT_TIMEOUT_MS_CONFIG);
+ }
Review Comment:
None of the configs in this class are dynamic so I am not sure that it is
worth it. We could perhaps mention it in the javadoc of the class. If we
introduce a dynamic config, we should indeed not use an attribute for it.
For the context, in KafkaConfig, we always had the distinction between val
(static values) and def (dynamic ones). We could do the same here, I suppose.
--
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
