rondagostino commented on a change in pull request #10008: URL: https://github.com/apache/kafka/pull/10008#discussion_r567495475
########## 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: Good question. We won't know what value to provide when using a Raft-based metadata quorum until we have caught up on the metadata log and know our configs. Providing it in the `startup()` function has no impact in the ZooKeeper world because we invoke that immediately after constructing the instance. In the Raft-based world, if we provide it in the constructor, it constrains us to not construct the instance until we have caught up and have our configs. By providing it in the `startup()` method it gives us additional flexibility to potentially instantiate the group coordinator but not yet "start" it. ---------------------------------------------------------------- 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