From aee70ef6fb968ebbd58c61ef8e134c6e0b3d645e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 18 Oct 2024 08:10:22 +0200 Subject: [PATCH 1/9] Propagate tree sizes when moving a node --- pkg/storage/fs/posix/tree/tree.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/storage/fs/posix/tree/tree.go b/pkg/storage/fs/posix/tree/tree.go index f41ab1dfb9..f648d9588f 100644 --- a/pkg/storage/fs/posix/tree/tree.go +++ b/pkg/storage/fs/posix/tree/tree.go @@ -334,11 +334,23 @@ func (t *Tree) Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) } } - err = t.Propagate(ctx, oldNode, 0) + // the size diff is the current treesize or blobsize of the old/source node + var sizeDiff int64 + if oldNode.IsDir(ctx) { + treeSize, err := oldNode.GetTreeSize(ctx) + if err != nil { + return err + } + sizeDiff = int64(treeSize) + } else { + sizeDiff = oldNode.Blobsize + } + + err = t.Propagate(ctx, oldNode, -sizeDiff) if err != nil { return errors.Wrap(err, "Decomposedfs: Move: could not propagate old node") } - err = t.Propagate(ctx, newNode, 0) + err = t.Propagate(ctx, newNode, sizeDiff) if err != nil { return errors.Wrap(err, "Decomposedfs: Move: could not propagate new node") } From 35f9e7c452c52aab3e4bf0217ddf153596fa4314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 31 Oct 2024 10:51:57 +0100 Subject: [PATCH 2/9] Remove unused parameter --- pkg/storage/fs/posix/tree/assimilation.go | 4 +-- .../posix/tree/gpfsfilauditloggingwatcher.go | 6 ++--- pkg/storage/fs/posix/tree/inotifywatcher.go | 25 ++++++++++--------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/pkg/storage/fs/posix/tree/assimilation.go b/pkg/storage/fs/posix/tree/assimilation.go index eca850ff4a..32af618d5a 100644 --- a/pkg/storage/fs/posix/tree/assimilation.go +++ b/pkg/storage/fs/posix/tree/assimilation.go @@ -160,14 +160,14 @@ func (t *Tree) workScanQueue() { } // Scan scans the given path and updates the id chache -func (t *Tree) Scan(path string, action EventAction, isDir bool, recurse bool) error { +func (t *Tree) Scan(path string, action EventAction, isDir bool) error { // cases: switch action { case ActionCreate: if !isDir { // 1. New file (could be emitted as part of a new directory) // -> assimilate file - // -> scan parent directory recursively + // -> scan parent directory recursively to update tree size and catch nodes that weren't covered by an event if !t.scanDebouncer.InProgress(filepath.Dir(path)) { t.scanDebouncer.Debounce(scanItem{ Path: path, diff --git a/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go b/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go index 5c8d8a201f..0dbd4f6dfc 100644 --- a/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go +++ b/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go @@ -64,15 +64,15 @@ start: } switch ev.Event { case "CREATE": - go func() { _ = w.tree.Scan(ev.Path, ActionCreate, false, false) }() + go func() { _ = w.tree.Scan(ev.Path, ActionCreate, false) }() case "CLOSE": bytesWritten, err := strconv.Atoi(ev.BytesWritten) if err == nil && bytesWritten > 0 { - go func() { _ = w.tree.Scan(ev.Path, ActionUpdate, false, true) }() + go func() { _ = w.tree.Scan(ev.Path, ActionUpdate, false) }() } case "RENAME": go func() { - _ = w.tree.Scan(ev.Path, ActionMove, false, true) + _ = w.tree.Scan(ev.Path, ActionMove, false) _ = w.tree.WarmupIDCache(ev.Path, false, false) }() } diff --git a/pkg/storage/fs/posix/tree/inotifywatcher.go b/pkg/storage/fs/posix/tree/inotifywatcher.go index 2858b851bc..ed6c75eb2a 100644 --- a/pkg/storage/fs/posix/tree/inotifywatcher.go +++ b/pkg/storage/fs/posix/tree/inotifywatcher.go @@ -45,18 +45,19 @@ func (iw *InotifyWatcher) Watch(path string) { if isLockFile(event.Filename) || isTrash(event.Filename) || iw.tree.isUpload(event.Filename) { continue } - switch e { - case inotifywaitgo.DELETE: - go func() { _ = iw.tree.HandleFileDelete(event.Filename) }() - case inotifywaitgo.CREATE: - go func() { _ = iw.tree.Scan(event.Filename, ActionCreate, event.IsDir, false) }() - case inotifywaitgo.MOVED_TO: - go func() { - _ = iw.tree.Scan(event.Filename, ActionMove, event.IsDir, true) - }() - case inotifywaitgo.CLOSE_WRITE: - go func() { _ = iw.tree.Scan(event.Filename, ActionUpdate, event.IsDir, true) }() - } + e := e // copy loop variable for use in goroutine + go func() { + switch e { + case inotifywaitgo.DELETE: + case inotifywaitgo.MOVED_FROM: + _ = iw.tree.Scan(event.Filename, ActionDelete, event.IsDir) + case inotifywaitgo.CREATE: + case inotifywaitgo.MOVED_TO: + _ = iw.tree.Scan(event.Filename, ActionCreate, event.IsDir) + case inotifywaitgo.CLOSE_WRITE: + _ = iw.tree.Scan(event.Filename, ActionUpdate, event.IsDir) + } + }() } case err := <-errors: From 496c482af5621010f53b00d530729af413bf79fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 6 Nov 2024 09:08:46 +0100 Subject: [PATCH 3/9] Fix treesizes when deleting an item --- pkg/storage/fs/posix/tree/assimilation.go | 7 +++++ .../fs/posix/tree/gpfswatchfolderwatcher.go | 31 ++++++++++--------- pkg/storage/fs/posix/tree/inotifywatcher.go | 1 - 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pkg/storage/fs/posix/tree/assimilation.go b/pkg/storage/fs/posix/tree/assimilation.go index 32af618d5a..9a98fecc60 100644 --- a/pkg/storage/fs/posix/tree/assimilation.go +++ b/pkg/storage/fs/posix/tree/assimilation.go @@ -218,6 +218,13 @@ func (t *Tree) Scan(path string, action EventAction, isDir bool) error { case ActionDelete: _ = t.HandleFileDelete(path) + // 6. Deleted file or directory + // -> update parent and all children + t.scanDebouncer.Debounce(scanItem{ + Path: filepath.Dir(path), + ForceRescan: true, + Recurse: true, + }) } return nil diff --git a/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go b/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go index e206e27284..5c3aec91bd 100644 --- a/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go +++ b/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go @@ -29,13 +29,13 @@ func (w *GpfsWatchFolderWatcher) Watch(topic string) { Topic: topic, }) - lwev := &lwe{} for { m, err := r.ReadMessage(context.Background()) if err != nil { break } + lwev := &lwe{} err = json.Unmarshal(m.Value, lwev) if err != nil { continue @@ -45,22 +45,25 @@ func (w *GpfsWatchFolderWatcher) Watch(topic string) { continue } - isDir := strings.Contains(lwev.Event, "IN_ISDIR") + go func() { + isDir := strings.Contains(lwev.Event, "IN_ISDIR") - switch { - case strings.Contains(lwev.Event, "IN_CREATE"): - go func() { _ = w.tree.Scan(lwev.Path, ActionCreate, isDir, false) }() + switch { + case strings.Contains(lwev.Event, "IN_DELETE"): + _ = w.tree.Scan(lwev.Path, ActionDelete, isDir) - case strings.Contains(lwev.Event, "IN_CLOSE_WRITE"): - bytesWritten, err := strconv.Atoi(lwev.BytesWritten) - if err == nil && bytesWritten > 0 { - go func() { _ = w.tree.Scan(lwev.Path, ActionUpdate, isDir, true) }() + case strings.Contains(lwev.Event, "IN_CREATE"): + _ = w.tree.Scan(lwev.Path, ActionCreate, isDir) + + case strings.Contains(lwev.Event, "IN_CLOSE_WRITE"): + bytesWritten, err := strconv.Atoi(lwev.BytesWritten) + if err == nil && bytesWritten > 0 { + _ = w.tree.Scan(lwev.Path, ActionUpdate, isDir) + } + case strings.Contains(lwev.Event, "IN_MOVED_TO"): + _ = w.tree.Scan(lwev.Path, ActionMove, isDir) } - case strings.Contains(lwev.Event, "IN_MOVED_TO"): - go func() { - _ = w.tree.Scan(lwev.Path, ActionMove, isDir, true) - }() - } + }() } if err := r.Close(); err != nil { log.Fatal("failed to close reader:", err) diff --git a/pkg/storage/fs/posix/tree/inotifywatcher.go b/pkg/storage/fs/posix/tree/inotifywatcher.go index ed6c75eb2a..b5e5d8f02b 100644 --- a/pkg/storage/fs/posix/tree/inotifywatcher.go +++ b/pkg/storage/fs/posix/tree/inotifywatcher.go @@ -49,7 +49,6 @@ func (iw *InotifyWatcher) Watch(path string) { go func() { switch e { case inotifywaitgo.DELETE: - case inotifywaitgo.MOVED_FROM: _ = iw.tree.Scan(event.Filename, ActionDelete, event.IsDir) case inotifywaitgo.CREATE: case inotifywaitgo.MOVED_TO: From 32194b9ff7a1b766433f3603608840a29bb72315 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 7 Nov 2024 08:26:09 +0100 Subject: [PATCH 4/9] Make sure to set the size to 0 for empty directories --- pkg/storage/fs/posix/tree/assimilation.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/storage/fs/posix/tree/assimilation.go b/pkg/storage/fs/posix/tree/assimilation.go index 9a98fecc60..33413d036f 100644 --- a/pkg/storage/fs/posix/tree/assimilation.go +++ b/pkg/storage/fs/posix/tree/assimilation.go @@ -600,6 +600,7 @@ func (t *Tree) WarmupIDCache(root string, assimilate, onlyDirty bool) error { if !dirty { return filepath.SkipDir } + sizes[path] += 0 // Make sure to set the size to 0 for empty directories } attribs, err := t.lookup.MetadataBackend().All(context.Background(), path) From 3d6e696ec0d133061523be73b0b80d8c384ad912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 7 Nov 2024 08:28:34 +0100 Subject: [PATCH 5/9] Handle MovedFrom events --- pkg/storage/fs/posix/tree/assimilation.go | 12 ++++++++++- .../posix/tree/gpfsfilauditloggingwatcher.go | 18 ++++++++++++++++ .../fs/posix/tree/gpfswatchfolderwatcher.go | 21 +++++++++++++++++++ pkg/storage/fs/posix/tree/inotifywatcher.go | 21 +++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/pkg/storage/fs/posix/tree/assimilation.go b/pkg/storage/fs/posix/tree/assimilation.go index 33413d036f..687eb5ca47 100644 --- a/pkg/storage/fs/posix/tree/assimilation.go +++ b/pkg/storage/fs/posix/tree/assimilation.go @@ -59,6 +59,7 @@ const ( ActionUpdate ActionMove ActionDelete + ActionMoveFrom ) type queueItem struct { @@ -216,9 +217,18 @@ func (t *Tree) Scan(path string, action EventAction, isDir bool) error { Recurse: isDir, }) + case ActionMoveFrom: + // 6. file/directory moved out of the watched directory + // -> update directory + if err := t.setDirty(filepath.Dir(path), true); err != nil { + return err + } + + go func() { _ = t.WarmupIDCache(filepath.Dir(path), false, true) }() + case ActionDelete: _ = t.HandleFileDelete(path) - // 6. Deleted file or directory + // 7. Deleted file or directory // -> update parent and all children t.scanDebouncer.Debounce(scanItem{ Path: filepath.Dir(path), diff --git a/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go b/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go index 0dbd4f6dfc..ead3cf398f 100644 --- a/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go +++ b/pkg/storage/fs/posix/tree/gpfsfilauditloggingwatcher.go @@ -1,3 +1,21 @@ +// Copyright 2018-2024 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + package tree import ( diff --git a/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go b/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go index 5c3aec91bd..5a0fb6a4bd 100644 --- a/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go +++ b/pkg/storage/fs/posix/tree/gpfswatchfolderwatcher.go @@ -1,3 +1,21 @@ +// Copyright 2018-2024 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + package tree import ( @@ -52,6 +70,9 @@ func (w *GpfsWatchFolderWatcher) Watch(topic string) { case strings.Contains(lwev.Event, "IN_DELETE"): _ = w.tree.Scan(lwev.Path, ActionDelete, isDir) + case strings.Contains(lwev.Event, "IN_MOVE_FROM"): + _ = w.tree.Scan(lwev.Path, ActionMoveFrom, isDir) + case strings.Contains(lwev.Event, "IN_CREATE"): _ = w.tree.Scan(lwev.Path, ActionCreate, isDir) diff --git a/pkg/storage/fs/posix/tree/inotifywatcher.go b/pkg/storage/fs/posix/tree/inotifywatcher.go index b5e5d8f02b..e5a1ade10e 100644 --- a/pkg/storage/fs/posix/tree/inotifywatcher.go +++ b/pkg/storage/fs/posix/tree/inotifywatcher.go @@ -1,3 +1,21 @@ +// Copyright 2018-2024 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + package tree import ( @@ -30,6 +48,7 @@ func (iw *InotifyWatcher) Watch(path string) { Events: []inotifywaitgo.EVENT{ inotifywaitgo.CREATE, inotifywaitgo.MOVED_TO, + inotifywaitgo.MOVED_FROM, inotifywaitgo.CLOSE_WRITE, inotifywaitgo.DELETE, }, @@ -50,6 +69,8 @@ func (iw *InotifyWatcher) Watch(path string) { switch e { case inotifywaitgo.DELETE: _ = iw.tree.Scan(event.Filename, ActionDelete, event.IsDir) + case inotifywaitgo.MOVED_FROM: + _ = iw.tree.Scan(event.Filename, ActionMoveFrom, event.IsDir) case inotifywaitgo.CREATE: case inotifywaitgo.MOVED_TO: _ = iw.tree.Scan(event.Filename, ActionCreate, event.IsDir) From 79b54c5e91896341d80dd10b19cce39f72f15ed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 7 Nov 2024 08:29:01 +0100 Subject: [PATCH 6/9] Update the id cache early when moving items --- pkg/storage/fs/posix/tree/tree.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/storage/fs/posix/tree/tree.go b/pkg/storage/fs/posix/tree/tree.go index f648d9588f..2ba94a88a7 100644 --- a/pkg/storage/fs/posix/tree/tree.go +++ b/pkg/storage/fs/posix/tree/tree.go @@ -310,6 +310,12 @@ func (t *Tree) Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) return errors.Wrap(err, "Decomposedfs: could not move child") } + // update the id cache + if newNode.ID == "" { + newNode.ID = oldNode.ID + } + _ = t.lookup.(*lookup.Lookup).CacheID(ctx, newNode.SpaceID, newNode.ID, filepath.Join(newNode.ParentPath(), newNode.Name)) + // rename the lock (if it exists) if _, err := os.Stat(oldNode.LockFilePath()); err == nil { err = os.Rename( @@ -321,11 +327,6 @@ func (t *Tree) Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) } } - // update the id cache - if newNode.ID == "" { - newNode.ID = oldNode.ID - } - _ = t.lookup.(*lookup.Lookup).CacheID(ctx, newNode.SpaceID, newNode.ID, filepath.Join(newNode.ParentPath(), newNode.Name)) // update id cache for the moved subtree. if oldNode.IsDir(ctx) { err = t.WarmupIDCache(filepath.Join(newNode.ParentPath(), newNode.Name), false, false) From 973a47df26a42273bb4ff5bde1182c578d9fdc7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 7 Nov 2024 09:23:16 +0100 Subject: [PATCH 7/9] Add changelog --- changelog/unreleased/improve-posixfs.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/improve-posixfs.md diff --git a/changelog/unreleased/improve-posixfs.md b/changelog/unreleased/improve-posixfs.md new file mode 100644 index 0000000000..529d3dc5a7 --- /dev/null +++ b/changelog/unreleased/improve-posixfs.md @@ -0,0 +1,6 @@ +Enhancement: Improve posixfs stability and performance + +The posixfs storage driver saw a number of bugfixes and optimizations. + +https://github.com/cs3org/reva/pull/4916 + From 84112ae766868b2bbf46b632dcd042e07dfe00ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 7 Nov 2024 09:55:45 +0100 Subject: [PATCH 8/9] Loop variables have per-iteration scope in go 1.22+ --- pkg/storage/fs/posix/tree/inotifywatcher.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/storage/fs/posix/tree/inotifywatcher.go b/pkg/storage/fs/posix/tree/inotifywatcher.go index e5a1ade10e..749c4cd489 100644 --- a/pkg/storage/fs/posix/tree/inotifywatcher.go +++ b/pkg/storage/fs/posix/tree/inotifywatcher.go @@ -64,7 +64,6 @@ func (iw *InotifyWatcher) Watch(path string) { if isLockFile(event.Filename) || isTrash(event.Filename) || iw.tree.isUpload(event.Filename) { continue } - e := e // copy loop variable for use in goroutine go func() { switch e { case inotifywaitgo.DELETE: From 9eaa5bbca75a360242d75184e864299adc8f4759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 7 Nov 2024 10:07:02 +0100 Subject: [PATCH 9/9] Fix merge error --- pkg/storage/fs/posix/tree/inotifywatcher.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/storage/fs/posix/tree/inotifywatcher.go b/pkg/storage/fs/posix/tree/inotifywatcher.go index 749c4cd489..54b7157e0f 100644 --- a/pkg/storage/fs/posix/tree/inotifywatcher.go +++ b/pkg/storage/fs/posix/tree/inotifywatcher.go @@ -70,8 +70,7 @@ func (iw *InotifyWatcher) Watch(path string) { _ = iw.tree.Scan(event.Filename, ActionDelete, event.IsDir) case inotifywaitgo.MOVED_FROM: _ = iw.tree.Scan(event.Filename, ActionMoveFrom, event.IsDir) - case inotifywaitgo.CREATE: - case inotifywaitgo.MOVED_TO: + case inotifywaitgo.CREATE, inotifywaitgo.MOVED_TO: _ = iw.tree.Scan(event.Filename, ActionCreate, event.IsDir) case inotifywaitgo.CLOSE_WRITE: _ = iw.tree.Scan(event.Filename, ActionUpdate, event.IsDir)