[GitHub] [kafka] gharris1727 commented on a diff in pull request #13287: MINOR: Refactor task change logic to AbstractHerder, reuse for standalone mode.

["gharris1727 (via GitHub)" <gi...@apache.org>]

gharris1727 commented on code in PR #13287:
URL: https://github.com/apache/kafka/pull/13287#discussion_r1120610035


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerder.java:
##########
@@ -427,9 +427,8 @@ private void updateConnectorTasks(String connName) {
         }
 
         List<Map<String, String>> newTaskConfigs = recomputeTaskConfigs(connName);
-        List<Map<String, String>> oldTaskConfigs = configState.allTaskConfigs(connName);
 
-        if (!newTaskConfigs.equals(oldTaskConfigs)) {

Review Comment:
   This logic is simpler, but only because `ClusterConfigState::allTaskConfigs` and `List::equals` are abstracting some of the complexity.
   
   There are multiple reasons I decided to keep the DistributedHerder implementation instead of the Standalone herder:
   
   1. The DistributedHerder implementation allows us to provide more detailed debug logs about how the task configs are different, in a way that `List::equals` hides. Since distributed mode is more common, I figured adding those logs to standalone was better than taking them away from distributed mode.
   2. The `taskConfig` and `allTaskConfigs` are duplicating one another to some degree (they're both retrieving task configs, they're both applying transformations) but we cannot remove `taskConfig` as it is used in lots of other areas of the herder.
   3. The `allTaskConfigs` is currently performing a naive search over all known task configs, and while we could fix that, it requires a larger change with higher regression risk.
   



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org