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

levels: Cleanup builder resources on building an empty table #1414

Merged
merged 1 commit into from
Jul 13, 2020

Conversation

damz
Copy link
Contributor

@damz damz commented Jul 10, 2020

The compaction process can leak *table.Builder instances if the table built is empty.

This used to be a big problem: before #1409 it would leak out the block compression/encryption goroutines (that are all blocked on reading from the b.blockChan, and because of that it would leak the builder too.

Since #1409 this is now a simple question of correctness. Take it or leave it.


This change is Reviewable

Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

:lgtm: This looks like a harmless change.

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)

@jarifibrahim jarifibrahim merged commit 8dd3fd9 into hypermodeinc:master Jul 13, 2020
jarifibrahim pushed a commit that referenced this pull request Oct 2, 2020
The compaction process can leak `*table.Builder` instances if the table built
is empty.

This used to be a big problem: before #1409 it would leak out the block
compression/encryption goroutines (that are all blocked on reading from the
`b.blockChan`, and because of that, it would leak the builder too.

Since #1409 this is now a simple question of correctness. Take it or leave it.
mYmNeo added a commit to mYmNeo/badger that referenced this pull request Jan 17, 2023
mYmNeo added a commit to mYmNeo/badger that referenced this pull request Feb 13, 2023
mYmNeo added a commit to mYmNeo/badger that referenced this pull request Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants