rondagostino commented on a change in pull request #10155:
URL: https://github.com/apache/kafka/pull/10155#discussion_r579240227
##########
File path: core/src/main/scala/kafka/server/BrokerServer.scala
##########
@@ -326,14 +326,16 @@ class BrokerServer(
// Start log manager, which will perform (potentially lengthy)
recovery-from-unclean-shutdown if required.
logManager.startup(metadataCache.getAllTopics())
// Start other services that we've delayed starting, in the appropriate
order.
- replicaManager.endMetadataChangeDeferral(
- RequestHandlerHelper.onLeadershipChange(groupCoordinator,
transactionCoordinator, _, _))
replicaManager.startup()
replicaManager.startHighWatermarkCheckPointThread()
groupCoordinator.startup(() =>
metadataCache.numPartitions(Topic.GROUP_METADATA_TOPIC_NAME).
getOrElse(config.offsetsTopicPartitions))
transactionCoordinator.startup(() =>
metadataCache.numPartitions(Topic.TRANSACTION_STATE_TOPIC_NAME).
getOrElse(config.transactionTopicPartitions))
+ // Apply deferred changes after starting coordinators to avoid
GroupMetadataManager IllegalStateException
+ // due to Kafka scheduler not yet running (this manifests if we have
deferred any coordinator offsets partitions).
Review comment:
> comments should explain the intended behavior vs what errors we are
trying to avoid
Yeah, makes sense. New comment is as follows:
```
// Apply deferred partition metadata changes after starting replica
manager and coordinators
// so that those services are ready and able to process the changes.
```
----------------------------------------------------------------
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:
[email protected]
