kpatelatwork commented on a change in pull request #10822:
URL: https://github.com/apache/kafka/pull/10822#discussion_r651296666



##########
File path: 
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java
##########
@@ -1063,6 +1076,104 @@ public int generation() {
         return generation;
     }
 
+    @Override
+    public void restartConnectorAndTasks(
+            RestartRequest request,
+            Callback<ConnectorStateInfo> callback
+    ) {
+        final String connectorName = request.connectorName();
+        addRequest(
+                () -> {
+                    if (checkRebalanceNeeded(callback)) {
+                        return null;
+                    }
+                    if 
(!configState.connectors().contains(request.connectorName())) {
+                        callback.onCompletion(new NotFoundException("Unknown 
connector: " + connectorName), null);
+                        return null;
+                    }
+                    if (isLeader()) {
+                        // Write a restart request to the config backing 
store, to be executed asynchronously in tick()
+                        configBackingStore.putRestartRequest(request);
+                        // Compute and send the response that this was accepted
+                        Optional<RestartPlan> maybePlan = 
buildRestartPlanFor(request);
+                        if (!maybePlan.isPresent()) {
+                            callback.onCompletion(new 
NotFoundException("Status for connector " + connectorName + " not found", 
null), null);
+                        } else {
+                            RestartPlan plan = maybePlan.get();
+                            callback.onCompletion(null, 
plan.restartConnectorStateInfo());
+                        }
+                    } else {
+                        callback.onCompletion(new NotLeaderException("Cannot 
process restart request since it is not assigned to this member", leaderUrl()), 
null);
+                    }
+
+                    return null;
+                },
+                forwardErrorCallback(callback)
+        );
+    }
+
+    /**
+     * Process all pending restart requests. There can be at most one request 
per connector, because of how
+     * {@link RestartRequest#equals(Object)} and {@link 
RestartRequest#hashCode()} are based only on the connector name.
+     *
+     * <p>This method is called from within the {@link #tick()} method. It is 
synchronized so that all pending restart requests
+     * are processed at once before any additional requests are added.
+     */
+    private synchronized void processRestartRequests() {

Review comment:
       We can't make a copy because we are doing pollFirst and its removing it 
out of set
   `        while ((request = pendingRestartRequests.pollFirst()) != null) {
   `
   
   The whole method synchronization was done deliberately to keep the code 
simple. One more point is that this just triggers the start of connector/tasks 
and real start happens in another thread so this should be pretty fast.




-- 
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:
us...@infra.apache.org


Reply via email to