ashoke-cube commented on code in PR #16095:
URL: https://github.com/apache/kafka/pull/16095#discussion_r1640754632
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConnector.java:
##########
@@ -231,6 +231,12 @@ private synchronized void onFailure(Throwable t) {
if (this.state == State.FAILED)
return;
+ // Call stop() on the connector to release its resources. Connector
+ // could fail in the start() method, which is why we call stop() on
+ // INIT state as well.
+ if (this.state == State.STARTED || this.state == State.INIT)
+ connector.stop();
Review Comment:
@gharris1727
I agree on the assessment that having `connector.stop()` inside `onFailure`
handler isn't right.
Both the proposed solutions involve modifications to `doShutDown()` method
to call `connector.stop()` when the state is FAILED. I will make that change.
However, the proposed change to trigger doShutdown() early when
transitioning to the FAILED state would move the connector to the STOPPED
state. This is a shift, as it introduces a new state transition that was not
previously possible. It could be misleading as well, as a connector in the
STOPPED state might not clearly signal that a failure occurred, compared to a
connector in the FAILED state. I am open to your suggestions here.
I think leaving the resources allocated when the connector is in the FAILED
state until a restart is fine.
--
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
