[GitHub] [kafka] mumrah opened a new pull request #10696: KAFKA-12777 Refactor and cleanup AutoTopicCreationManager

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

mumrah opened a new pull request #10696:
URL: https://github.com/apache/kafka/pull/10696


   Rather than using multiple optional class members to determine if we are in ZK or KRaft mode, use inheritance. The factory method in AutoTopicCreationManager now makes the decision about which mode we're in and provides only the needed dependencies to the concrete classes (no optionals).
   
   ```scala
   object AutoTopicCreationManager {
     def apply(
       config: KafkaConfig,
       channelManager: BrokerToControllerChannelManager,
       metadataSupport: MetadataSupport,
       groupCoordinator: GroupCoordinator,
       txnCoordinator: TransactionCoordinator,
     ): AutoTopicCreationManager = {
       metadataSupport match {
         case zk: ZkSupport => new ZkAutoTopicCreationManager(config, zk, groupCoordinator, txnCoordinator)
         case _: RaftSupport => new DefaultAutoTopicCreationManager(config, channelManager, groupCoordinator, txnCoordinator)
       }
     }
   }
   ```
   
   This also adds some error handling to the response handler when running in KRaft mode (KAFKA-12777).


-- 
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

[GitHub] [kafka] cmccabe commented on pull request #10696: KAFKA-12777 Refactor and cleanup AutoTopicCreationManager

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

cmccabe commented on pull request #10696:
URL: https://github.com/apache/kafka/pull/10696#issuecomment-842547338


   (Copying offline discussion to github)
   
   I think creating a base class and using implementation inheritance is worse than the current code. Looking at the code, I see very little commonality between the ZK implementation and the API-based implementation. I think we should just have an interface and two separate implementations.
   
   The main code that we'd want to share between the two implementations is the routine that checks if topics have the wrong names. That's a very small amount of code which could simply be in a static method. If you want you could combine that with checking if a change is "in-flight" by passing in a map from topics to booleans, or similar. Or just have two separate maps in the separate implementations.


-- 
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