Skip to content

KAFKA-9830: Backport "Implement AutoCloseable in ErrorReporter and subclasses"#8594

Closed
gharris1727 wants to merge 2 commits intoapache:2.1from
gharris1727:dlq-leak-producer-21
Closed

KAFKA-9830: Backport "Implement AutoCloseable in ErrorReporter and subclasses"#8594
gharris1727 wants to merge 2 commits intoapache:2.1from
gharris1727:dlq-leak-producer-21

Conversation

@gharris1727
Copy link
Copy Markdown
Contributor

This is a backport of #8442 for the 2.0 and 2.1 branches.

In these versions, producer.close(Duration) does not exist, and producer.close(long, TimeUnit) is not deprecated. The tests are adjusted to use the older API.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

…pache#8442)

* The DeadLetterQueueReporter has a KafkaProducer that it must close to clean up resources
* Currently, the producer and its threads are leaked every time a task is stopped
* Responsibility for cleaning up ErrorReporters is transitively assigned to the
    ProcessingContext, RetryWithToleranceOperator, and WorkerSinkTask/WorkerSinkTask classes
* One new unit test in ErrorReporterTest asserts that the producer is closed by the dlq reporter

Reviewers: Arjun Satish <arjun@confluent.io>, Chris Egerton <chrise@confluent.io>, Chia-Ping Tsai <chia7712@gmail.com>, Konstantine Karantasis <konstantine@confluent.io>
…ed here

Signed-off-by: Greg Harris <gregh@confluent.io>
@gharris1727
Copy link
Copy Markdown
Contributor Author

cc @kkonstantine for review.

@kkonstantine
Copy link
Copy Markdown
Contributor

Backporting #8442 up to 2.2 seems actually sufficient for this issue (not CVE/data loss related). Therefore, I'll go ahead and close this PR. We may reopen if we find evidence that such backporting is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants