gharris1727 commented on PR #14762:
URL: https://github.com/apache/kafka/pull/14762#issuecomment-1896474948
I think due to the locking concerns I raised earlier, and that we can
resolve this resource leak in our tests, this PR is not viable to merge. We can
revisit this in the future if
gharris1727 closed pull request #14762: KAFKA-15826: Close consumer when sink
task is cancelled
URL: https://github.com/apache/kafka/pull/14762
--
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
gharris1727 commented on PR #14762:
URL: https://github.com/apache/kafka/pull/14762#issuecomment-1850986158
I found this same leak again when debugging some failures of the
`kafka.utils.TestUtils.verifyNoUnexpectedThreads` assertion in core tests.
Whenever we leak one of these clients in
gharris1727 commented on PR #14762:
URL: https://github.com/apache/kafka/pull/14762#issuecomment-1816905171
> What was the conclusion here? Is triggering wakeup on a separate thread
(through WorkerSinkTask::stop) before also eventually closing the consumer on a
separate thread (through
yashmayya commented on code in PR #14762:
URL: https://github.com/apache/kafka/pull/14762#discussion_r1397042412
##
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerSinkTask.java:
##
@@ -163,6 +163,12 @@ public void initialize(TaskConfig taskConfig) {
gharris1727 commented on PR #14762:
URL: https://github.com/apache/kafka/pull/14762#issuecomment-1815495283
Hey @yashmayya Thanks for letting me know the original intent of the test. I
agree, changing the test to expect the call to succeed was not in the spirit of
the original test, so
yashmayya commented on PR #14762:
URL: https://github.com/apache/kafka/pull/14762#issuecomment-1814288871
> Since other workers in the cluster may not have this fix applied, I'll
leave the error message unchanged and just update the assertions.
Hm, the original purpose of those tests
gharris1727 commented on PR #14762:
URL: https://github.com/apache/kafka/pull/14762#issuecomment-1813403690
The only problem I have with this approach is that the [official
documentation](https://kafka.apache.org/36/javadoc/org/apache/kafka/clients/consumer/KafkaConsumer.html#multithreaded)
gharris1727 commented on PR #14762:
URL: https://github.com/apache/kafka/pull/14762#issuecomment-1812953889
@yashmayya I noticed those failures and wasn't sure they're affected, thanks
for explaining exactly what is going on!
Since other workers in the cluster may not have this fix
gharris1727 opened a new pull request, #14762:
URL: https://github.com/apache/kafka/pull/14762
Currently the only place that the consumer is closed is on the task thread,
which may be blocked indefinitely by the task plugin. Similar to
AbstractWorkerSourceTask, we should close the consumer
10 matches
Mail list logo
