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

Recycle goroutine doing compaction #119

Closed
cyriltovena opened this issue Jul 4, 2022 · 4 comments · Fixed by #231
Closed

Recycle goroutine doing compaction #119

cyriltovena opened this issue Jul 4, 2022 · 4 comments · Fixed by #231

Comments

@cyriltovena
Copy link
Contributor

While working on #118 I realised that we create a go-routine per compaction for each granule.

I think this should be bounded but also we should probably recycle go-routine since those can grow pretty large in term of memory

image

I suggest we create 8 go-routines that will take care of compactions, then send granule to compact via a buffered channel of 32, meaning it can block on the write path.

@brancz
Copy link
Member

brancz commented Jul 5, 2022

I think it's important that we keep the write path non-blocking/lock-free, so I wonder if we could find another way. Maybe we could just mark the granule to need compaction and have the compaction go routines just cycle through the granules to pick up the ones that need compaction? Happy for other suggestions but not spawning a go routine at every insert is obviously not a long term option (I think we always knew this but just punted on that when we initially wrote this).

@thorfour
Copy link
Contributor

Yea, when I initially wrote that I played around with buffered channels as well as a mark and sweep style. This one performed the best, and until we better understood the characteristics of how it performed decided to defer solving this.

@brancz
Copy link
Member

brancz commented Jul 19, 2022

Potentially wild idea, what if we used a sync pool for reusing goroutines?

@brancz
Copy link
Member

brancz commented Jul 19, 2022

Never mind we would end up leaking goroutines.

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 a pull request may close this issue.

3 participants