rondagostino commented on code in PR #13759:
URL: https://github.com/apache/kafka/pull/13759#discussion_r1210468300


##########
metadata/src/main/java/org/apache/kafka/controller/BrokerHeartbeatManager.java:
##########
@@ -223,6 +283,21 @@ public BrokerHeartbeatState next() {
         }
     }
 
+    /**
+     * The maximum number of timed out heartbeats to count.
+     */
+    static final int DEFAULT_TIMED_OUT_HEARTBEAT_COUNT_MAX = 1000;
+
+    /**
+     * The time period over which to track timed out heartbeats.
+     */
+    static final long DEFAULT_TIMED_OUT_HEARTBEAT_COUNT_WINDOW_NS = 
TimeUnit.MINUTES.toNanos(5);
+
+    /**
+     * The number of heartbeats to notice missing before we go into overload.
+     */
+    static final int DEFAULT_TIMED_OUT_HEARTBEAT_OVERLOAD_THRESHOLD = 3;
+

Review Comment:
   > As a side note, broker session is not 18 seconds. It is 9 seconds.
   
   I was confused by the existence of this, which sets 18 seconds: 
https://github.com/apache/kafka/blob/d557f7997359daa5853d446d9093995577540de2/metadata/src/main/java/org/apache/kafka/controller/ClusterControlManager.java#L77
   
   But the above always get overridden by the value of 
`broker.session.timeout.ms` in the config, which does default to 9 seconds 
(https://github.com/apache/kafka/blob/d557f7997359daa5853d446d9093995577540de2/core/src/main/scala/kafka/server/KafkaConfig.scala#L83).
   
   So yes, it is 9 seconds as you state above.  I wonder if the default in 
`ClusterControlManager.java` should be changed to match the config's default.
   
   



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