Skip to content

Commit

Permalink
satellite/gcbf: fix data race in TestGCBFUseRangedLoop
Browse files Browse the repository at this point in the history
Satellites[0].GCBF is already started when testplanet boots up,
so calling Run separately ends up causing a data race.

Instead create a new instance, that should avoid this issue.

Fixes #6435

Change-Id: I6603ef63da7a6ab8bdb952cf5aaca17eb0392e2c
  • Loading branch information
egonelbre committed Nov 1, 2023
1 parent 0a4a109 commit 9e0fff5
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
8 changes: 4 additions & 4 deletions satellite/gc-bf.go
Expand Up @@ -91,14 +91,14 @@ func NewGarbageCollectionBF(log *zap.Logger, db DB, metabaseDB *metabase.DB, rev
peer.GarbageCollection.Config = config.GarbageCollectionBF

var observer rangedloop.Observer
if config.GarbageCollectionBF.UseSyncObserver {
if peer.GarbageCollection.Config.UseSyncObserver {
observer = bloomfilter.NewSyncObserver(log.Named("gc-bf"),
config.GarbageCollectionBF,
peer.GarbageCollection.Config,
peer.Overlay.DB,
)
} else {
observer = bloomfilter.NewObserver(log.Named("gc-bf"),
config.GarbageCollectionBF,
peer.GarbageCollection.Config,
peer.Overlay.DB,
)
}
Expand All @@ -109,7 +109,7 @@ func NewGarbageCollectionBF(log *zap.Logger, db DB, metabaseDB *metabase.DB, rev
observer,
})

if !config.GarbageCollectionBF.RunOnce {
if !peer.GarbageCollection.Config.RunOnce {
peer.Services.Add(lifecycle.Item{
Name: "garbage-collection-bf",
Run: peer.RangedLoop.Service.Run,
Expand Down
30 changes: 23 additions & 7 deletions satellite/gc-bf_test.go
Expand Up @@ -7,23 +7,39 @@ import (
"testing"

"github.com/stretchr/testify/require"
"go.uber.org/zap"

"storj.io/common/testcontext"
"storj.io/storj/private/revocation"
"storj.io/storj/private/testplanet"
"storj.io/storj/satellite"
)

func TestGCBFUseRangedLoop(t *testing.T) {
testplanet.Run(t, testplanet.Config{
SatelliteCount: 1,
Reconfigure: testplanet.Reconfigure{
Satellite: func(log *zap.Logger, index int, config *satellite.Config) {
config.GarbageCollectionBF.RunOnce = true
},
},
}, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) {
err := planet.Satellites[0].GCBF.Run(ctx)
config := planet.Satellites[0].Config

revocationDB, err := revocation.OpenDBFromCfg(ctx, config.Server.Config)
require.NoError(t, err)
defer ctx.Check(revocationDB.Close)

config.GarbageCollectionBF.RunOnce = true

gcbf, err := satellite.NewGarbageCollectionBF(
planet.Log().Named("test-gcbf"),
// hopefully we can share the databases
planet.Satellites[0].GCBF.DB,
planet.Satellites[0].Metabase.DB,
revocationDB,
planet.NewVersionInfo(),
&config,
nil,
)
require.NoError(t, err)
defer ctx.Check(gcbf.Close)

err = gcbf.Run(ctx)
require.NoError(t, err)
})
}

0 comments on commit 9e0fff5

Please sign in to comment.