[GitHub] [kafka] cmccabe commented on a change in pull request #10008: MINOR: Remove ZK dependency for __consumer_offsets partition count

[GitBox <gi...@apache.org>]

cmccabe commented on a change in pull request #10008:
URL: https://github.com/apache/kafka/pull/10008#discussion_r568009986



##########
File path: core/src/main/scala/kafka/server/KafkaServer.scala
##########
@@ -318,8 +318,9 @@ class KafkaServer(
 
         /* start group coordinator */
         // Hardcode Time.SYSTEM for now as some Streams tests fail otherwise, it would be good to fix the underlying issue
-        groupCoordinator = GroupCoordinator(config, zkClient, replicaManager, Time.SYSTEM, metrics)
-        groupCoordinator.startup()
+        groupCoordinator = GroupCoordinator(config, replicaManager, Time.SYSTEM, metrics)
+        groupCoordinator.startup(zkClient.getTopicPartitionCount(Topic.GROUP_METADATA_TOPIC_NAME).

Review comment:
       > What's the advantage of having a coordinator that is not started?
   
   This is a pattern that we are using for a lot of components in the kip-500 branch.  It reflects the fact that in the kip-500 world, we have a "before metadata is available" state and an "after metadata is available" state.  The advantage is that it makes this state explicit rather than being implicit.  When the state is implicit mistakes can easily happen-- for example, someone asks for some piece of metadata that is not available.  The original author of the code may have "just known" this, but it's better to formalize it so that people don't make mistakes.
   
   I think the callback approach is really clunky and tends to obfuscate the code.  What's going on here is really that the number of partitions gets initialized once.  Having a complex callback chain just obscures that and makes it tough to read.




----------------------------------------------------------------
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:
users@infra.apache.org