Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix goroutine leak and close Computation.Event computation finishes #15

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

pellared
Copy link
Contributor

@pellared pellared commented Apr 24, 2024

Fixes #9.

Fixes computation shutdown.

Before the fix each computation is causing a goroutine leak.

eventChBuffer was never closed.
This was causing go bufferMessages(comp.eventChBuffer, comp.eventCh) to never finish (goroutine leak) and also the channel returned by Computation.Event() was never closed.

@pellared pellared requested review from a team as code owners April 24, 2024 07:25
@pellared pellared requested a review from a team as a code owner April 24, 2024 07:30
@pellared pellared changed the title Fix Computation.Event to be closed when computation completes Fix Computation.Event to be closed when computation is done Apr 24, 2024
@pellared pellared changed the title Fix Computation.Event to be closed when computation is done Fix Computation.Event to be closed when computation finishes Apr 24, 2024
@pellared pellared changed the title Fix Computation.Event to be closed when computation finishes Fix goroutine leak and Computation.Event to be closed when computation finishes Apr 24, 2024
@pellared pellared changed the title Fix goroutine leak and Computation.Event to be closed when computation finishes Fix goroutine leak and close Computation.Event computation finishes Apr 24, 2024
@pellared
Copy link
Contributor Author

pellared commented Apr 24, 2024

@benkeith-splunk, PTAL.

I think it may be useful for https://github.com/signalfx/signalfx-k8s-metrics-adapter

Copy link

@benkeith-splunk benkeith-splunk left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.

@pellared pellared merged commit 1be78a9 into main Apr 24, 2024
5 checks passed
@pellared pellared deleted the links branch April 24, 2024 15:29
@github-actions github-actions bot locked and limited conversation to collaborators Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky test: TestMultipleComputations
3 participants