TrueCloudLab/frostfs-node
19
1
Fork 27
Code Issues 91 Pull requests 7 Projects Releases 1 Activity Actions

[#576] Set SyncWrites for badger writecache by default #582

Merged
fyrchik merged 1 commit from ale64bit/frostfs-node:fix/576-wc-badger-sync-writes into master 2023-08-09 10:16:33 +00:00
Conversation 0 Commits 1 Files changed 3 +1 -13
3 changed files with 1 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
cmd/frostfs-node/config.go
View file

@ -734,7 +734,6 @@ func (c *cfg) getWriteCacheOpts(shCfg shardCfg) writecacheconfig.Options {
writecachebadger.WithMaxObjectSize(wcRead.maxObjSize),
writecachebadger.WithFlushWorkersCount(wcRead.flushWorkerCount),
writecachebadger.WithMaxCacheSize(wcRead.sizeLimit),
writecachebadger.WithNoSync(wcRead.noSync),
writecachebadger.WithLogger(c.log),
writecachebadger.WithGCInterval(wcRead.gcInterval),
)

12
pkg/local_object_storage/writecache/writecachebadger/options.go
View file

@ -47,8 +47,6 @@ type options struct {
maxCacheSize uint64
// objCounters contains atomic counters for the number of objects stored in cache.
objCounters counters
// noSync is true iff FSTree allows unsynchronized writes.
noSync bool
// reportError is the function called when encountering disk errors in background workers.
reportError func(string, error)
// metrics is metrics implementation
@ -109,16 +107,6 @@ func WithMaxCacheSize(sz uint64) Option {
}
}
// WithNoSync sets an option to allow returning to caller on PUT before write is persisted.
// Note, that we use this flag for FSTree only and DO NOT use it for a bolt DB because
// we cannot yet properly handle the corrupted database during the startup. This SHOULD NOT
// be relied upon and may be changed in future.
func WithNoSync(noSync bool) Option {
fyrchik commented 2023-08-09 09:05:02 +00:00
Review
Copy link

Why remove this setting? It could be useful to get some baselines, we can just use WithSyncWrites(!opts.noSync)

Why remove this setting? It _could_ be useful to get some baselines, we can just use `WithSyncWrites(!opts.noSync)`
ale64bit commented 2023-08-09 09:10:59 +00:00
Review
Copy link

This is only used for fstree in the bbolt cache. There's no fstree in badger cache so it seems quite misleading to have the same parameter control different settings for each cache, especially one that can lead to subtle mistakes.

This is only used for fstree in the bbolt cache. There's no fstree in badger cache so it seems quite misleading to have the same parameter control different settings for each cache, especially one that can lead to subtle mistakes.
👍 1
return func(o *options) {
o.noSync = noSync
}
}
// WithReportErrorFunc sets error reporting function.
func WithReportErrorFunc(f func(string, error)) Option {
return func(o *options) {

1
pkg/local_object_storage/writecache/writecachebadger/util.go
View file

@ -11,6 +11,7 @@ import (
func OpenDB(p string, ro bool, l *logger.Logger) (*badger.DB, error) {
return badger.Open(badger.DefaultOptions(p).
WithReadOnly(ro).
WithSyncWrites(true).
WithLoggingLevel(badger.ERROR).
WithLogger(badgerLoggerWrapper{l}))
}