-
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
feat: Add SetNewReadersBlocked for purger.purge #26114
base: master-1.x
Are you sure you want to change the base?
Conversation
I'm wanting to add a test that has contention between filestore.Apply and filestore.Replace where there are multiple TSM files being Ref'ed and Unref'ed while attempting to run them through the purger. I'm struggling to be able to write proper test cases for this.
tsdb/engine/tsm1/file_store_test.go
Outdated
dir := MustTempDir() | ||
defer os.RemoveAll(dir) |
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.
Could you use t.TempDir()
instead?
tsdb/engine/tsm1/file_store_test.go
Outdated
fs.Apply(func(r tsm1.TSMFile) error { | ||
overlapsTimeRangeMinMaxLock.Lock() | ||
time.Sleep(time.Duration(rand.Intn(5)) * time.Millisecond) | ||
overlapsTimeRangeMinMaxLock.Unlock() | ||
return nil | ||
}) |
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.
The use of fs.Apply
to inject mischief is really smart. However, the overlapsTimeRangeMinMaxLock
just makes the goroutines launched by fs.Apply
run in a serial manner, equivalent to having the limiter in fs.Apply
only allow 1 goroutine at a time. Given that, I'm not the mutex helps create contention more than just doing time.Sleep
.
WIP