From abd502215f0fc0c8e6454eeeeb54b9c83c680dc4 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Thu, 8 Feb 2024 18:04:18 +0300 Subject: [PATCH] [#970] fstree: Move file locking to the generic writer It is not a part of FSTree itself, but rather a way to solve concurrent counter update on non-linux implementations. New linux implementations is pretty simple: link fails when the file exists, unlink fails when the file doesn't exist. Signed-off-by: Evgenii Stratonikov --- .../blobstor/fstree/counter.go | 5 +++ .../blobstor/fstree/fstree.go | 9 ++-- .../blobstor/fstree/fstree_write_generic.go | 45 ++++++++++++------- .../blobstor/fstree/option.go | 3 -- 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/pkg/local_object_storage/blobstor/fstree/counter.go b/pkg/local_object_storage/blobstor/fstree/counter.go index 70b34609..718104e2 100644 --- a/pkg/local_object_storage/blobstor/fstree/counter.go +++ b/pkg/local_object_storage/blobstor/fstree/counter.go @@ -18,6 +18,11 @@ func (c *noopCounter) Set(uint64) {} func (c *noopCounter) Inc() {} func (c *noopCounter) Dec() {} +func counterEnabled(c FileCounter) bool { + _, noop := c.(*noopCounter) + return !noop +} + type SimpleCounter struct { v atomic.Uint64 } diff --git a/pkg/local_object_storage/blobstor/fstree/fstree.go b/pkg/local_object_storage/blobstor/fstree/fstree.go index ba691b05..76ce5bf1 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree.go @@ -53,9 +53,7 @@ type FSTree struct { readOnly bool metrics Metrics - fileGuard keyLock - fileCounter FileCounter - fileCounterEnabled bool + fileCounter FileCounter writer writer } @@ -88,14 +86,13 @@ func New(opts ...Option) *FSTree { Depth: 4, DirNameLen: DirNameLen, metrics: &noopMetrics{}, - fileGuard: &noopKeyLock{}, fileCounter: &noopCounter{}, log: &logger.Logger{Logger: zap.L()}, } for i := range opts { opts[i](f) } - f.writer = newGenericWriteData(f) + f.writer = newGenericWriteData(f.fileCounter, f.Permissions, f.noSync) return f } @@ -444,7 +441,7 @@ func (t *FSTree) GetRange(ctx context.Context, prm common.GetRangePrm) (common.G // initFileCounter walks the file tree rooted at FSTree's root, // counts total items count, inits counter and returns number of stored objects. func (t *FSTree) initFileCounter() error { - if !t.fileCounterEnabled { + if !counterEnabled(t.fileCounter) { return nil } diff --git a/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go b/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go index e1e3a53b..80807695 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go @@ -10,6 +10,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/common" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/util/logicerr" + utilSync "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/sync" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" ) @@ -22,19 +23,31 @@ type genericWriter struct { perm fs.FileMode flags int - t *FSTree - suffix atomic.Uint64 + fileGuard keyLock + fileCounter FileCounter + fileCounterEnabled bool + suffix atomic.Uint64 } -func newGenericWriteData(t *FSTree) writer { +func newGenericWriteData(c FileCounter, perm fs.FileMode, noSync bool) writer { flags := os.O_WRONLY | os.O_CREATE | os.O_TRUNC | os.O_EXCL - if !t.noSync { + if !noSync { flags |= os.O_SYNC } + + var fileGuard keyLock = &noopKeyLock{} + fileCounterEnabled := counterEnabled(c) + if fileCounterEnabled { + fileGuard = utilSync.NewKeyLocker[string]() + } + var w = &genericWriter{ - perm: t.Permissions, + perm: perm, flags: flags, - t: t, + + fileCounterEnabled: fileCounterEnabled, + fileGuard: fileGuard, + fileCounter: c, } return w } @@ -46,9 +59,9 @@ func (w *genericWriter) writeData(p string, data []byte) error { // writeAndRename opens tmpPath exclusively, writes data to it and renames it to p. func (w *genericWriter) writeAndRename(tmpPath, p string, data []byte) error { - if w.t.fileCounterEnabled { - w.t.fileGuard.Lock(p) - defer w.t.fileGuard.Unlock(p) + if w.fileCounterEnabled { + w.fileGuard.Lock(p) + defer w.fileGuard.Unlock(p) } err := w.writeFile(tmpPath, data) @@ -64,15 +77,15 @@ func (w *genericWriter) writeAndRename(tmpPath, p string, data []byte) error { return err } - if w.t.fileCounterEnabled { - w.t.fileCounter.Inc() + if w.fileCounterEnabled { + w.fileCounter.Inc() var targetFileExists bool if _, e := os.Stat(p); e == nil { targetFileExists = true } err = os.Rename(tmpPath, p) if err == nil && targetFileExists { - w.t.fileCounter.Dec() + w.fileCounter.Dec() } } else { err = os.Rename(tmpPath, p) @@ -96,12 +109,12 @@ func (w *genericWriter) writeFile(p string, data []byte) error { func (w *genericWriter) removeFile(p string) error { var err error - if w.t.fileCounterEnabled { - w.t.fileGuard.Lock(p) + if w.fileCounterEnabled { + w.fileGuard.Lock(p) err = os.Remove(p) - w.t.fileGuard.Unlock(p) + w.fileGuard.Unlock(p) if err == nil { - w.t.fileCounter.Dec() + w.fileCounter.Dec() } } else { err = os.Remove(p) diff --git a/pkg/local_object_storage/blobstor/fstree/option.go b/pkg/local_object_storage/blobstor/fstree/option.go index e6c7a260..4d1f8fc2 100644 --- a/pkg/local_object_storage/blobstor/fstree/option.go +++ b/pkg/local_object_storage/blobstor/fstree/option.go @@ -4,7 +4,6 @@ import ( "io/fs" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger" - utilSync "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/sync" "go.uber.org/zap" ) @@ -48,9 +47,7 @@ func WithMetrics(m Metrics) Option { func WithFileCounter(c FileCounter) Option { return func(f *FSTree) { - f.fileCounterEnabled = true f.fileCounter = c - f.fileGuard = utilSync.NewKeyLocker[string]() } }