smjn commented on code in PR #22292:
URL: https://github.com/apache/kafka/pull/22292#discussion_r3257281502


##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -724,17 +725,14 @@ class BrokerServer(
   private def createShareStatePersister(): Persister = {
     if (config.shareGroupConfig.shareGroupPersisterClassName.nonEmpty) {
       val klass = 
Utils.loadClass(config.shareGroupConfig.shareGroupPersisterClassName, 
classOf[Object]).asInstanceOf[Class[Persister]]
-
       if (klass.getName.equals(classOf[DefaultStatePersister].getName)) {
-        klass.getConstructor(classOf[PersisterStateManager])
-          .newInstance(
-            new PersisterStateManager(
-              NetworkUtils.buildNetworkClient("Persister", config, metrics, 
Time.SYSTEM, new LogContext(s"[Persister broker=${config.brokerId}]")),
-              new ShareCoordinatorMetadataCacheHelperImpl(metadataCache, key 
=> shareCoordinator.partitionFor(key), config.interBrokerListenerName, 
groupConfigManager),
-              Time.SYSTEM,
-              shareGroupTimer
-            )
-          )
+        klass.getDeclaredMethod("instance", classOf[KafkaClient], 
classOf[ShareCoordinatorMetadataCacheHelper], classOf[Time], classOf[Timer])

Review Comment:
   Hi,
   initially the persister was planned to be a pluggable component and code was 
written in that way but it was subsequently decided to make it factory 
pluggable. Hence, the if condition. But yes with current state of code, this is 
unnecessary, will rectify



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to