Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

contrib/confluentinc/confluent-kafka-go: fix goroutine leak in Produce #2924

Merged
merged 6 commits into from
Oct 18, 2024

Conversation

rarguelloF
Copy link
Contributor

@rarguelloF rarguelloF commented Oct 11, 2024

What does this PR do?

(Thanks to @majorgreys who reported this issue).

Fix a goroutine leak in producer.Produce when the actual Produce call was returning an error and a deliveryChan is provided.
In this case, the goroutine "wrapping" the original channel gets stuck waiting for a message in the channel, but no event is ever sent to that channel.

Motivation

Fix goroutine leak.

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@github-actions github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Oct 11, 2024
@rarguelloF rarguelloF marked this pull request as ready for review October 11, 2024 15:19
@rarguelloF rarguelloF requested review from a team as code owners October 11, 2024 15:19
@pr-commenter
Copy link

pr-commenter bot commented Oct 11, 2024

Benchmarks

Benchmark execution time: 2024-10-11 15:53:48

Comparing candidate commit 8899d7f in PR branch rarguelloF/kafka-goroutine-leak with baseline commit 5b9a8af in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 58 metrics, 1 unstable metrics.

Copy link

@majorgreys majorgreys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇🏽

@darccio darccio merged commit 3935c78 into main Oct 18, 2024
144 of 145 checks passed
@darccio darccio deleted the rarguelloF/kafka-goroutine-leak branch October 18, 2024 15:23
darccio added a commit that referenced this pull request Oct 23, 2024
#2924)

Co-authored-by: Dario Castañé <dario.castane@datadoghq.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants