jeffkbkim commented on code in PR #13322:
URL: https://github.com/apache/kafka/pull/13322#discussion_r1122374811


##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -163,6 +163,20 @@ object Defaults {
   val GroupInitialRebalanceDelayMs = 3000
   val GroupMaxSize: Int = Int.MaxValue
 
+  /** New group coordinator configs */
+  val NewGroupCoordinatorEnable = false
+  val GroupCoordinatorNumThreads = 1
+
+  /** Consumer group configs */
+  val ConsumerGroupSessionTimeoutMs = 45000
+  val ConsumerGroupMinSessionTimeoutMs = 45000
+  val ConsumerGroupMaxSessionTimeoutMs = 60000
+  val ConsumerGroupHeartbeatIntervalMs = 5000
+  val ConsumerGroupMinHeartbeatInternalMs = 5000
+  val ConsumerGroupMaxHeartbeatInternalMs = 15000
+  val ConsumerGroupMaxSize = Int.MaxValue
+  val ConsumerGroupAssignors = ""

Review Comment:
   KIP 848 lists
   ```
   org.apache.kafka.server.group.consumer.UniformAssignor, 
org.apache.kafka.server.group.consumer.RangeAssignor
   ```
   as the default. are we waiting until these two are implemented?



##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -163,6 +163,20 @@ object Defaults {
   val GroupInitialRebalanceDelayMs = 3000
   val GroupMaxSize: Int = Int.MaxValue
 
+  /** New group coordinator configs */
+  val NewGroupCoordinatorEnable = false
+  val GroupCoordinatorNumThreads = 1
+
+  /** Consumer group configs */
+  val ConsumerGroupSessionTimeoutMs = 45000
+  val ConsumerGroupMinSessionTimeoutMs = 45000
+  val ConsumerGroupMaxSessionTimeoutMs = 60000
+  val ConsumerGroupHeartbeatIntervalMs = 5000
+  val ConsumerGroupMinHeartbeatInternalMs = 5000
+  val ConsumerGroupMaxHeartbeatInternalMs = 15000

Review Comment:
   should these be "Interval"?



##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -483,11 +497,27 @@ object KafkaConfig {
   val ControlledShutdownMaxRetriesProp = "controlled.shutdown.max.retries"
   val ControlledShutdownRetryBackoffMsProp = 
"controlled.shutdown.retry.backoff.ms"
   val ControlledShutdownEnableProp = "controlled.shutdown.enable"
+
   /** ********* Group coordinator configuration ***********/
   val GroupMinSessionTimeoutMsProp = "group.min.session.timeout.ms"
   val GroupMaxSessionTimeoutMsProp = "group.max.session.timeout.ms"
   val GroupInitialRebalanceDelayMsProp = "group.initial.rebalance.delay.ms"
   val GroupMaxSizeProp = "group.max.size"
+
+  /** New group coordinator configs */
+  val NewGroupCoordinatorEnableProp = "group.coordinator.new.enable"
+  val GroupCoordinatorNumThreadsProp = "group.coordinator.threads"
+
+  /** Consumer group configs */
+  val ConsumerGroupSessionTimeoutMsProp = "group.consumer.session.timeout.ms"
+  val ConsumerGroupMinSessionTimeoutMsProp = 
"group.consumer.min.session.timeout.ms"
+  val ConsumerGroupMaxSessionTimeoutMsProp = 
"group.consumer.max.session.timeout.ms"
+  val ConsumerGroupHeartbeatIntervalMsProp = 
"group.consumer.heartbeat.interval.ms"
+  val ConsumerGroupMinHeartbeatInternalMsProp = 
"group.consumer.min.heartbeat.interval.ms"
+  val ConsumerGroupMaxHeartbeatInternalMsProp 
="group.consumer.max.heartbeat.interval.ms"

Review Comment:
   should these also be "Interval"?



##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -1267,6 +1314,24 @@ object KafkaConfig {
       .define(GroupInitialRebalanceDelayMsProp, INT, 
Defaults.GroupInitialRebalanceDelayMs, MEDIUM, GroupInitialRebalanceDelayMsDoc)
       .define(GroupMaxSizeProp, INT, Defaults.GroupMaxSize, atLeast(1), 
MEDIUM, GroupMaxSizeDoc)
 
+      /** New group coordinator configs */
+      // All properties are kept internal until KIP-848 is released.
+      // This property meant to be here only during the development of 
KIP-848. It will
+      // be replaced by a metadata version before releasing it.
+      .defineInternal(NewGroupCoordinatorEnableProp, BOOLEAN, 
Defaults.NewGroupCoordinatorEnable, null, MEDIUM, NewGroupCoordinatorEnableDoc)
+      .defineInternal(GroupCoordinatorNumThreadsProp, INT, 
Defaults.GroupCoordinatorNumThreads, atLeast(1), MEDIUM, 
GroupCoordinatorNumThreadsDoc)
+
+      /** Consumer groups configs */
+      // All properties are kept internal until KIP-848 is released.
+      .defineInternal(ConsumerGroupSessionTimeoutMsProp, INT, 
Defaults.ConsumerGroupSessionTimeoutMs, atLeast(1), MEDIUM, 
ConsumerGroupSessionTimeoutMsDoc)

Review Comment:
   how does the operator use ConsumerGroupSessionTimeoutMs and 
ConsumerGroupSessionMin/MaxTimeoutMs configs?
   
   is the idea to set min/max once, then configure the base timeout which the 
min/max would enforce? 



##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -1267,6 +1314,24 @@ object KafkaConfig {
       .define(GroupInitialRebalanceDelayMsProp, INT, 
Defaults.GroupInitialRebalanceDelayMs, MEDIUM, GroupInitialRebalanceDelayMsDoc)
       .define(GroupMaxSizeProp, INT, Defaults.GroupMaxSize, atLeast(1), 
MEDIUM, GroupMaxSizeDoc)
 
+      /** New group coordinator configs */
+      // All properties are kept internal until KIP-848 is released.
+      // This property meant to be here only during the development of 
KIP-848. It will

Review Comment:
   nit: "property is meant"



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

Reply via email to