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


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConnector.java:
##########
@@ -307,7 +307,7 @@ void doShutdown() {
                                     + " as the connector has been scheduled 
for shutdown"),
                         null);
             }
-            if (state == State.STARTED)
+            if (state == State.STARTED || state == State.FAILED)
                 connector.stop();

Review Comment:
   connector.stop can fail, and if that happens then we don't transit to 
STOPPED, we re-transit to FAILED, but have a different error. I think this will 
shadow the exception that caused the connector to fail in the first place, 
which is almost certainly more confusing than helpful.
   
   In the catch clause, can you swap out the state/statusListener for 
onFailure? It has failure-deduplication logic that seems useful here.



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

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

Reply via email to