cmccabe commented on code in PR #14369:
URL: https://github.com/apache/kafka/pull/14369#discussion_r1391770284
##########
core/src/main/scala/kafka/server/ReplicaManager.scala:
##########
@@ -269,7 +270,9 @@ class ReplicaManager(val config: KafkaConfig,
delayedRemoteFetchPurgatoryParam:
Option[DelayedOperationPurgatory[DelayedRemoteFetch]] = None,
threadNamePrefix: Option[String] = None,
val brokerEpochSupplier: () => Long = () => -1,
- addPartitionsToTxnManager:
Option[AddPartitionsToTxnManager] = None
+ addPartitionsToTxnManager:
Option[AddPartitionsToTxnManager] = None,
+ assignmentManager: Option[AssignmentsManager] = None,
Review Comment:
I know this is a common pattern in `ReplicaManager` but it just seems so
bad. We don't really care about the details of `AssignmentsManager` or
`BrokerLifecycleManager` here. Shouldn't we just be passing a reference to an
interface like `AssignmentHandler`?
```
interface AssignmentHandler {
void onAssignment(TopicIdPartition, Uuid);
void propagateDirectoryFailure(Uuid directoryId);
}
```
etc.
Then we can initialize a dummy version by default, to keep all the unit
tests working without changes (if desired).
--
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
