dielhennr commented on a change in pull request #9101:
URL: https://github.com/apache/kafka/pull/9101#discussion_r465329379



##########
File path: core/src/main/scala/kafka/server/DynamicConfig.scala
##########
@@ -103,12 +122,25 @@ object DynamicConfig {
       .define(ProducerByteRateOverrideProp, LONG, DefaultProducerOverride, 
MEDIUM, ProducerOverrideDoc)
       .define(ConsumerByteRateOverrideProp, LONG, DefaultConsumerOverride, 
MEDIUM, ConsumerOverrideDoc)
       .define(RequestPercentageOverrideProp, DOUBLE, DefaultRequestOverride, 
MEDIUM, RequestOverrideDoc)
+      .define(AcksOverrideProp, STRING, DefaultAcksOverride, 
ConfigDef.ValidString.in("all", "-1", "0", "1"), HIGH, AcksOverrideDoc)
+      .define(
+        SessionTimeoutOverrideProp, 
+        INT, 
+        DefaultSessionTimeoutOverride, 
+        ConfigDef.Range.between(Defaults.GroupMinSessionTimeoutMs, 
Defaults.GroupMaxSessionTimeoutMs), 
+        HIGH, 
+        SessionTimeoutOverrideDoc)
+      .define(HeartbeatIntervalOverrideProp, INT, 
DefaultHeartbeatIntervalOverride, HIGH, HeartbeatIntervalOverrideDoc)

Review comment:
       This prevents key-value pairs that the broker knows are invalid from 
being persisted to zookeeper e.g. acks=2. The group coordinator also requires 
that a group members session timeout is in between GroupMinSessionTimeoutMs and 
GroupMaxSessionTimeoutMs, so the broker can also prevent invalid session 
timeouts from being persisted to zk.
   
   The client will still have to validate them against the user-provided client 
configs. For example, if the dynamic config is acks=1 but the user enabled 
idempotence the dynamic config should be ignored by the client. Another example 
is that heartbeat.interval.ms must be less than session.timeout.ms. This 
validation needs to be done by the client since session.timeout.ms could be 
dynamically configured while heartbeat.interval.ms is not.
   
   The motivation behind doing validation on the broker and the client is that 
some clients may accept the dynamic configs while others could reject them as 
long as the broker sanity-checks them before persisting to zk. If acks=2 is 
persisted it is invalid for all clients not a subset of them.




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to