TrueCloudLab/frostfs-node
18
1
Fork 27
Code Issues 90 Pull requests 3 Projects Releases 1 Activity Actions

Fix Badger writecache race #765

Merged
fyrchik merged 1 commit from dstepanov-yadro/frostfs-node:fix/wc_badger_race into master 2023-10-31 10:01:28 +00:00
Conversation 0 Commits 1 Files changed 2 +24 -13
2 changed files with 24 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
pkg/local_object_storage/writecache/writecachebadger/cachebadger.go
View file

@ -85,6 +85,9 @@ func (c *cache) DumpInfo() writecache.Info {
// Open opens and initializes database. Reads object counters from the ObjectCounters instance.
func (c *cache) Open(_ context.Context, readOnly bool) error {
c.modeMtx.Lock()
fyrchik commented 2023-10-30 14:38:04 +00:00
Outdated
Review
Copy link

There we races in SetMode, is it fixed with this?

There we races in `SetMode`, is it fixed with this?
dstepanov-yadro commented 2023-10-30 15:39:02 +00:00
Outdated
Review
Copy link

Hm, SetMode is already protected by mutex.

WARNING: DATA RACE
Write at 0x00c0001a2378 by goroutine 861:
  git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger.(*cache).openStore()
      /workspace/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger/storage.go:53 +0x145

Previous read at 0x00c0001a2378 by goroutine 1244:
  git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger.(*cache).runGCLoop.func1()
      /workspace/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger/gc.go:33 +0x2a8

I've missed fix for GC, so now it's ok.

Hm, `SetMode` is already protected by mutex. ``` WARNING: DATA RACE Write at 0x00c0001a2378 by goroutine 861: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger.(*cache).openStore() /workspace/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger/storage.go:53 +0x145 Previous read at 0x00c0001a2378 by goroutine 1244: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger.(*cache).runGCLoop.func1() /workspace/TrueCloudLab/frostfs-node/pkg/local_object_storage/writecache/writecachebadger/gc.go:33 +0x2a8 ``` I've missed fix for GC, so now it's ok.
defer c.modeMtx.Unlock()
err := c.openStore(readOnly)
if err != nil {
return metaerr.Wrap(err)
@ -94,6 +97,9 @@ func (c *cache) Open(_ context.Context, readOnly bool) error {
// Init runs necessary services.
func (c *cache) Init() error {
c.modeMtx.Lock()
defer c.modeMtx.Unlock()
c.log.Info(logs.WritecacheBadgerInitExperimental)
c.metrics.SetMode(c.mode)
ctx, cancel := context.WithCancel(context.Background())

31
pkg/local_object_storage/writecache/writecachebadger/gc.go
View file

@ -21,20 +21,25 @@ func (c *cache) runGCLoop(ctx context.Context) {
case <-ctx.Done():
return
case <-t.C:
// This serves to synchronize the c.db field when changing mode as well.
c.modeMtx.RLock()
ro := c.readOnly()
c.modeMtx.RUnlock()
if ro {
continue
}
// 0.5 is the recommended value so that write amplification of the value log is 2.
// See https://pkg.go.dev/github.com/dgraph-io/badger/v4#DB.RunValueLogGC for more info.
for c.db.RunValueLogGC(0.5) == nil {
c.log.Debug(logs.WritecacheDBValueLogGCRunCompleted)
}
c.runGC()
}
}
}()
}
func (c *cache) runGC() {
// This serves to synchronize the c.db field when changing mode as well.
c.modeMtx.RLock()
defer c.modeMtx.RUnlock()
ro := c.readOnly()
if ro {
return
}
// 0.5 is the recommended value so that write amplification of the value log is 2.
// See https://pkg.go.dev/github.com/dgraph-io/badger/v4#DB.RunValueLogGC for more info.
for c.db.RunValueLogGC(0.5) == nil {
c.log.Debug(logs.WritecacheDBValueLogGCRunCompleted)
}
}