-
Notifications
You must be signed in to change notification settings - Fork 36
fix test race condition #9
Conversation
Also stop exposing functions that don't do what they claim to do: * Invalidate does, technically invalidate. However, it's not threadsafe. * Rebuild doesn't *clear* the filter so it's only useful for the initial build. It's also not threadsafe. We can restore these functions later if we need them (but we'll have to change a few things to make them work properly). Also adds a `Wait` function to allow waiting for the bloom filter to finish building. fixes #6
case <-ctx.Done(): | ||
return ctx.Err() | ||
case <-b.buildChan: | ||
return b.buildErr |
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.
This way we have a way of getting at these errors.
@@ -102,7 +98,7 @@ func TestHasIsBloomCached(t *testing.T) { | |||
} | |||
|
|||
if float64(cacheFails)/float64(1000) > float64(0.05) { | |||
t.Fatal("Bloom filter has cache miss rate of more than 5%") | |||
t.Fatalf("Bloom filter has cache miss rate of more than 5%%") |
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.
Go vet wasn't happy.
// This chan is only used for testing to wait for bloom to enable | ||
rebuildChan chan struct{} | ||
blockstore Blockstore | ||
bloom *bloom.Bloom |
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.
Put the atomic first because go can be stupid about things like this.
"Total number of requests to bloom cache").Counter(), | ||
buildChan: make(chan struct{}), | ||
} | ||
go func() { |
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.
Might as well do this all in one goroutine.
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.
Thanks for fixing this.
Also stop exposing functions that don't do what they claim to do:
It's also not threadsafe.
We can restore these functions later if we need them (but we'll have to change a
few things to make them work properly).
Also adds a
Wait
function to allow waiting for the bloom filter to finishbuilding.
fixes #6