From 7f692409cf1a25167446f9094d8ba7b8bd678f8e Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Thu, 8 Feb 2024 11:57:23 +0300 Subject: [PATCH] [#970] fstree: Handle unsupported O_TMPFILE Metabase test relied on this behaviour, so fix the test too. Cherry-picking was hard and did too many conflicts, here is an original PR: https://github.com/nspcc-dev/neofs-node/pull/2624 Signed-off-by: Evgenii Stratonikov --- .../blobstor/fstree/control.go | 7 +++ .../blobstor/fstree/fstree.go | 4 +- .../blobstor/fstree/fstree_write_generic.go | 58 +++++++++++-------- .../blobstor/fstree/fstree_write_linux.go | 46 ++++++++++++--- .../blobstor/fstree/fstree_write_specific.go | 9 +++ .../shard/control_test.go | 10 +++- 6 files changed, 97 insertions(+), 37 deletions(-) create mode 100644 pkg/local_object_storage/blobstor/fstree/fstree_write_specific.go diff --git a/pkg/local_object_storage/blobstor/fstree/control.go b/pkg/local_object_storage/blobstor/fstree/control.go index c56312d38..b126f8d29 100644 --- a/pkg/local_object_storage/blobstor/fstree/control.go +++ b/pkg/local_object_storage/blobstor/fstree/control.go @@ -16,6 +16,13 @@ func (t *FSTree) Init() error { if err := util.MkdirAllX(t.RootPath, t.Permissions); err != nil { return err } + if !t.readOnly { + f := newSpecificWriteData(t.fileCounter, t.RootPath, t.Permissions, t.noSync) + if f != nil { + t.writeData = f + } + } + return t.initFileCounter() } diff --git a/pkg/local_object_storage/blobstor/fstree/fstree.go b/pkg/local_object_storage/blobstor/fstree/fstree.go index 7bb8db5da..fc29d4eaa 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree.go @@ -10,7 +10,6 @@ import ( "path/filepath" "strconv" "strings" - "sync/atomic" "syscall" "time" @@ -58,7 +57,7 @@ type FSTree struct { fileCounter FileCounter fileCounterEnabled bool - suffix atomic.Uint64 + writeData func(string, []byte) error } // Info groups the information about file storage. @@ -96,6 +95,7 @@ func New(opts ...Option) *FSTree { for i := range opts { opts[i](f) } + f.writeData = newGenericWriteData(f) return f } 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 0882be0dd..3d65808ec 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go @@ -1,5 +1,3 @@ -//go:build !linux - package fstree import ( @@ -7,24 +5,46 @@ import ( "io/fs" "os" "strconv" + "sync/atomic" "syscall" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/common" ) -func (t *FSTree) writeData(p string, data []byte) error { - tmpPath := p + "#" + strconv.FormatUint(t.suffix.Add(1), 10) - return t.writeAndRename(tmpPath, p, data) +type genericWriter struct { + perm fs.FileMode + flags int + + t *FSTree + suffix atomic.Uint64 +} + +func newGenericWriteData(t *FSTree) func(string, []byte) error { + flags := os.O_WRONLY | os.O_CREATE | os.O_TRUNC | os.O_EXCL + if !t.noSync { + flags |= os.O_SYNC + } + var w = &genericWriter{ + perm: t.Permissions, + flags: flags, + t: t, + } + return w.writeData +} + +func (w *genericWriter) writeData(p string, data []byte) error { + tmpPath := p + "#" + strconv.FormatUint(w.suffix.Add(1), 10) + return w.writeAndRename(tmpPath, p, data) } // writeAndRename opens tmpPath exclusively, writes data to it and renames it to p. -func (t *FSTree) writeAndRename(tmpPath, p string, data []byte) error { - if t.fileCounterEnabled { - t.fileGuard.Lock(p) - defer t.fileGuard.Unlock(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) } - err := t.writeFile(tmpPath, data) + err := w.writeFile(tmpPath, data) if err != nil { var pe *fs.PathError if errors.As(err, &pe) { @@ -37,15 +57,15 @@ func (t *FSTree) writeAndRename(tmpPath, p string, data []byte) error { return err } - if t.fileCounterEnabled { - t.fileCounter.Inc() + if w.t.fileCounterEnabled { + w.t.fileCounter.Inc() var targetFileExists bool if _, e := os.Stat(p); e == nil { targetFileExists = true } err = os.Rename(tmpPath, p) if err == nil && targetFileExists { - t.fileCounter.Dec() + w.t.fileCounter.Dec() } } else { err = os.Rename(tmpPath, p) @@ -53,18 +73,10 @@ func (t *FSTree) writeAndRename(tmpPath, p string, data []byte) error { return err } -func (t *FSTree) writeFlags() int { - flags := os.O_WRONLY | os.O_CREATE | os.O_TRUNC | os.O_EXCL - if t.noSync { - return flags - } - return flags | os.O_SYNC -} - // writeFile writes data to a file with path p. // The code is copied from `os.WriteFile` with minor corrections for flags. -func (t *FSTree) writeFile(p string, data []byte) error { - f, err := os.OpenFile(p, t.writeFlags(), t.Permissions) +func (w *genericWriter) writeFile(p string, data []byte) error { + f, err := os.OpenFile(p, w.flags, w.perm) if err != nil { return err } diff --git a/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go b/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go index edb7f0dce..3416cf7ac 100644 --- a/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go +++ b/pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go @@ -4,26 +4,52 @@ package fstree import ( "errors" + "io/fs" "strconv" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/common" "golang.org/x/sys/unix" ) -func (t *FSTree) writeData(p string, data []byte) error { - err := t.writeFile(p, data) +type linuxWriter struct { + root string + perm uint32 + flags int + + counter FileCounter +} + +func newSpecificWriteData(c FileCounter, root string, perm fs.FileMode, noSync bool) func(string, []byte) error { + flags := unix.O_WRONLY | unix.O_TMPFILE | unix.O_CLOEXEC + if !noSync { + flags |= unix.O_DSYNC + } + fd, err := unix.Open(root, flags, uint32(perm)) + if err != nil { + // Which means that OS-specific writeData can't be created + // and FSTree should use the generic one. + return nil + } + _ = unix.Close(fd) // Don't care about error. + w := &linuxWriter{ + root: root, + perm: uint32(perm), + flags: flags, + counter: c, + } + return w.writeData +} + +func (w *linuxWriter) writeData(p string, data []byte) error { + err := w.writeFile(p, data) if errors.Is(err, unix.ENOSPC) { return common.ErrNoSpace } return err } -func (t *FSTree) writeFile(p string, data []byte) error { - flags := unix.O_WRONLY | unix.O_TMPFILE | unix.O_CLOEXEC - if !t.noSync { - flags |= unix.O_DSYNC - } - fd, err := unix.Open(t.RootPath, flags, uint32(t.Permissions)) +func (w *linuxWriter) writeFile(p string, data []byte) error { + fd, err := unix.Open(w.root, w.flags, w.perm) if err != nil { return err } @@ -32,8 +58,10 @@ func (t *FSTree) writeFile(p string, data []byte) error { if err == nil { if n == len(data) { err = unix.Linkat(unix.AT_FDCWD, tmpPath, unix.AT_FDCWD, p, unix.AT_SYMLINK_FOLLOW) + if err == nil { + w.counter.Inc() + } if errors.Is(err, unix.EEXIST) { - // https://github.com/nspcc-dev/neofs-node/issues/2563 err = nil } } else { diff --git a/pkg/local_object_storage/blobstor/fstree/fstree_write_specific.go b/pkg/local_object_storage/blobstor/fstree/fstree_write_specific.go new file mode 100644 index 000000000..16b630b6a --- /dev/null +++ b/pkg/local_object_storage/blobstor/fstree/fstree_write_specific.go @@ -0,0 +1,9 @@ +//go:build !linux + +package fstree + +import "io/fs" + +func newSpecificWriteData(_ FileCounter, _ string, _ fs.FileMode, _ bool) func(string, []byte) error { + return nil +} diff --git a/pkg/local_object_storage/shard/control_test.go b/pkg/local_object_storage/shard/control_test.go index 4d7354595..d08747e13 100644 --- a/pkg/local_object_storage/shard/control_test.go +++ b/pkg/local_object_storage/shard/control_test.go @@ -2,6 +2,7 @@ package shard import ( "context" + "fmt" "io/fs" "math" "os" @@ -11,7 +12,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/object" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/common" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/fstree" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/blobstor/teststore" meta "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase" @@ -145,8 +145,12 @@ func TestRefillMetabaseCorrupted(t *testing.T) { require.NoError(t, sh.Close()) addr := object.AddressOf(obj) - _, err = fsTree.Put(context.Background(), common.PutPrm{Address: addr, RawData: []byte("not an object")}) - require.NoError(t, err) + // This is copied from `fstree.treePath()` to avoid exporting function just for tests. + { + saddr := addr.Object().EncodeToString() + "." + addr.Container().EncodeToString() + p := fmt.Sprintf("%s/%s/%s", fsTree.RootPath, saddr[:2], saddr[2:]) + require.NoError(t, os.WriteFile(p, []byte("not an object"), fsTree.Permissions)) + } sh = New( WithID(NewIDFromBytes([]byte{})),