-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Prevent race condition caused by WaitGroup re-use #9335
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I don't have a reliable repro case for this, but if the panic still occurs, we'll find out.
wg sync.WaitGroup // waitgroup for active level compaction goroutines | ||
done chan struct{} // channel to signal level compactions to stop | ||
levelWorkers int // Number of "workers" that expect compactions to be in a disabled state | ||
wg *sync.WaitGroup // waitgroup for active level compaction goroutines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to read the code pretty closely to understand why wg
is kept as a field even though it's passed through some methods as arguments. The comment here or in enableLevelCompactions
could probably explain it better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully this does the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
This has been an elusive issue, but this should fix the data race seen occasionally during testing.