You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "lucliu1108 (via GitHub)" <gi...@apache.org> on 2026/09/24 16:31:03 UTC
[PR] KAFKA-21154: Worker#startTask should close the instances when encountering error [kafka]
lucliu1108 opened a new pull request, #23583:
URL: https://github.com/apache/kafka/pull/23583
## Summary
This PR closes the converter wrappers properly in the case of an exception for Worker#startTask.
--
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
Re: [PR] KAFKA-21154: Worker#startTask should close the instances when encountering error [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #23583:
URL: https://github.com/apache/kafka/pull/23583
--
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
Re: [PR] KAFKA-21154: Worker#startTask should close the instances when encountering error [kafka]
Posted by "lucliu1108 (via GitHub)" <gi...@apache.org>.
lucliu1108 commented on code in PR #23583:
URL: https://github.com/apache/kafka/pull/23583#discussion_r4112285097
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java:
##########
@@ -681,6 +681,12 @@ private boolean startTask(
connectorStatusMetricsGroup.recordTaskAdded(id);
final ClassLoader connectorLoader;
+ Converter keyConverter = null;
Review Comment:
Created 2 helper methods: one for key/value converter plugin and another for header converter plugin. It is possible to do just one using supplier but the `newConverter` code will become chunky and hard to read, so I keep them as 2.
--
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
Re: [PR] KAFKA-21154: Worker#startTask should close the instances when encountering error [kafka]
Posted by "lucliu1108 (via GitHub)" <gi...@apache.org>.
lucliu1108 commented on PR #23583:
URL: https://github.com/apache/kafka/pull/23583#issuecomment-5818788037
> ## Copilot review overview
> ### 🔵 Needs a closer look
> Converter instances can still leak when initialization or configuration fails inside the plugin factory.
>
> **Review effort:** Lite **Findings:** None
https://github.com/apache/kafka/pull/23516 The plugin leak itself is already included in this PR
--
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
Re: [PR] KAFKA-21154: Worker#startTask should close the instances when encountering error [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23583:
URL: https://github.com/apache/kafka/pull/23583#discussion_r4111917127
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java:
##########
@@ -681,6 +681,12 @@ private boolean startTask(
connectorStatusMetricsGroup.recordTaskAdded(id);
final ClassLoader connectorLoader;
+ Converter keyConverter = null;
Review Comment:
This style is hard on the eyes. Would you mind adding a helper method to create the converter? The helper should wrap the converter with a Plugin directly and take care of closing the raw converter if the wrap fails, so callers only ever need to close the Plugin.
--
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
Re: [PR] KAFKA-21154: Worker#startTask should close the instances when encountering error [kafka]
Posted by "lucliu1108 (via GitHub)" <gi...@apache.org>.
lucliu1108 commented on code in PR #23583:
URL: https://github.com/apache/kafka/pull/23583#discussion_r4112285097
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java:
##########
@@ -681,6 +681,12 @@ private boolean startTask(
connectorStatusMetricsGroup.recordTaskAdded(id);
final ClassLoader connectorLoader;
+ Converter keyConverter = null;
Review Comment:
Created 2 helper methods: one for key/value converter plugin and another for header converter plugin. It is possible to do just one using supplier but the `newConverterPlugin` code will become chunky and hard to read, so I keep them separate.
--
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