[GitHub] [kafka] yashmayya commented on a diff in pull request #13434: KAFKA-14785: Connect offset read REST API

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

yashmayya commented on code in PR #13434:
URL: https://github.com/apache/kafka/pull/13434#discussion_r1155236415


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java:
##########
@@ -866,4 +867,18 @@ public List<ConfigKeyInfo> connectorPluginConfig(String pluginName) {
         }
     }
 
+    @Override
+    public void connectorOffsets(String connName, Callback<ConnectorOffsets> cb) {

Review Comment:
   Gotcha, minimising / avoiding concurrent calls to the `Worker` does seem like a worthwhile property to preserve. I've overridden the `AbstractHerder::connectorOffsets` method in `DistributedHerder` to put it through the tick thread and in `StandaloneHerder` to make it a synchronized method. I haven't added an additional call to `ConfigBackingStore::refresh` since we're capturing a fresh snapshot (in the `AbstractHerder`) and not using an existing stale one so we should be reasonably up to speed on the state of the topic (due to the work thread in the underlying Kafka based log). Btw on a side note, we don't seem to be making calls to `ConfigBackingStore::refresh` even in methods like `putConnectorConfig` where it does seem like it would be useful to do so, what do you think?



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