cadonna commented on code in PR #14873:
URL: https://github.com/apache/kafka/pull/14873#discussion_r1416983368
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManager.java:
##########
@@ -103,6 +103,12 @@ public class HeartbeatRequestManager implements
RequestManager {
*/
private final BackgroundEventHandler backgroundEventHandler;
+ /**
+ * Timer for tracking the time since the last consumer poll. If the timer
expires, the consumer will stop
+ * sending heartbeat until the next poll.
+ */
Review Comment:
Oh, I was not aware of the static membership handling in KIP-848. Should
have also looked into that instead of only looking at the legacy consumer.
Thanks for the clarification!
Yes, I agree that consumers that use group management should always send a
leave group as KIP-848 describes.
So the comment is wrong. I would propose to just remove the whole comment
and give the timer a descriptive name. In general I find inline comments
problematic because -- at some point -- they start to lie because they are
never properly maintained. Instead I prefer to use meaningful and descriptive
names and to extract code that need a comment to methods that either have a
meaningful name or a comment on the method, preferring the meaningful name.
What I just wrote applies in most cases but not in all, but it definitely
applies here.
--
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
