omkreddy commented on code in PR #16268:
URL: https://github.com/apache/kafka/pull/16268#discussion_r1633540628


##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -1455,6 +1487,30 @@ class KafkaConfig private(doLog: Boolean, val props: 
java.util.Map[_, _], dynami
       s"${GroupCoordinatorConfig.CONSUMER_GROUP_SESSION_TIMEOUT_MS_CONFIG} 
must be less than or equals " +
       s"to 
${GroupCoordinatorConfig.CONSUMER_GROUP_MAX_SESSION_TIMEOUT_MS_CONFIG}")
 
+    require(shareGroupMaxHeartbeatIntervalMs >= 
shareGroupMinHeartbeatIntervalMs,
+      s"${ShareGroupConfigs.SHARE_GROUP_MAX_HEARTBEAT_INTERVAL_MS_CONFIG} must 
be greater than or equals " +
+        s"to 
${ShareGroupConfigs.SHARE_GROUP_MIN_HEARTBEAT_INTERVAL_MS_CONFIG}")
+    require(shareGroupHeartbeatIntervalMs >= shareGroupMinHeartbeatIntervalMs,
+      s"${ShareGroupConfigs.SHARE_GROUP_HEARTBEAT_INTERVAL_MS_CONFIG} must be 
greater than or equals " +
+        s"to 
${ShareGroupConfigs.SHARE_GROUP_MIN_HEARTBEAT_INTERVAL_MS_CONFIG}")
+    require(shareGroupHeartbeatIntervalMs <= shareGroupMaxHeartbeatIntervalMs,
+      s"${ShareGroupConfigs.SHARE_GROUP_HEARTBEAT_INTERVAL_MS_CONFIG} must be 
less than or equals " +
+        s"to 
${ShareGroupConfigs.SHARE_GROUP_MAX_HEARTBEAT_INTERVAL_MS_CONFIG}")
+
+    require(shareGroupMaxSessionTimeoutMs >= shareGroupMinSessionTimeoutMs,
+      s"${ShareGroupConfigs.SHARE_GROUP_MAX_SESSION_TIMEOUT_MS_CONFIG} must be 
greater than or equals " +
+        s"to ${ShareGroupConfigs.SHARE_GROUP_MIN_SESSION_TIMEOUT_MS_CONFIG}")
+    require(shareGroupSessionTimeoutMs >= shareGroupMinSessionTimeoutMs,
+      s"${ShareGroupConfigs.SHARE_GROUP_SESSION_TIMEOUT_MS_CONFIG} must be 
greater than or equals " +
+        s"to ${ShareGroupConfigs.SHARE_GROUP_MIN_SESSION_TIMEOUT_MS_CONFIG}")
+    require(shareGroupSessionTimeoutMs <= shareGroupMaxSessionTimeoutMs,
+      s"${ShareGroupConfigs.SHARE_GROUP_SESSION_TIMEOUT_MS_CONFIG} must be 
less than or equals " +
+        s"to ${ShareGroupConfigs.SHARE_GROUP_MAX_SESSION_TIMEOUT_MS_CONFIG}")
+
+    require(shareGroupMaxRecordLockDurationMs >= 
shareGroupRecordLockDurationMs,

Review Comment:
   Dont we need to add check for 
`SHARE_GROUP_MIN_RECORD_LOCK_DURATION_MS_CONFIG`  similar to above checks for 
`shareGroupMaxSessionTimeoutMs` 



##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -292,6 +292,23 @@ object KafkaConfig {
       .define(GroupCoordinatorConfig.CONSUMER_GROUP_ASSIGNORS_CONFIG, LIST, 
GroupCoordinatorConfig.CONSUMER_GROUP_ASSIGNORS_DEFAULT, null, MEDIUM, 
GroupCoordinatorConfig.CONSUMER_GROUP_ASSIGNORS_DOC)
       .define(GroupCoordinatorConfig.CONSUMER_GROUP_MIGRATION_POLICY_CONFIG, 
STRING, GroupCoordinatorConfig.CONSUMER_GROUP_MIGRATION_POLICY_DEFAULT, 
CaseInsensitiveValidString.in(Utils.enumOptions(classOf[ConsumerGroupMigrationPolicy]):
 _*), MEDIUM, GroupCoordinatorConfig.CONSUMER_GROUP_MIGRATION_POLICY_DOC)
 
+      /** Share Group Configurations **/
+      // Internal configuration used by integration and system tests.
+      .defineInternal(ShareGroupConfigs.SHARE_GROUP_ENABLE_CONFIG, BOOLEAN, 
ShareGroupConfigs.SHARE_GROUP_ENABLE_DEFAULT, null, MEDIUM, 
ShareGroupConfigs.SHARE_GROUP_ENABLE_DOC)
+      .define(ShareGroupConfigs.SHARE_GROUP_DELIVERY_COUNT_LIMIT_CONFIG, INT, 
ShareGroupConfigs.SHARE_GROUP_DELIVERY_COUNT_LIMIT_DEFAULT, between(2, 10), 
MEDIUM, ShareGroupConfigs.SHARE_GROUP_DELIVERY_COUNT_LIMIT_DOC)
+      .define(ShareGroupConfigs.SHARE_GROUP_RECORD_LOCK_DURATION_MS_CONFIG, 
INT, ShareGroupConfigs.SHARE_GROUP_RECORD_LOCK_DURATION_MS_DEFAULT, 
between(1000, 60000), MEDIUM, 
ShareGroupConfigs.SHARE_GROUP_RECORD_LOCK_DURATION_MS_DOC)

Review Comment:
    `between(1000, 60000)` -This should be `atleast(` validator and add require 
check below validation checks?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to