From 39c3868c49e54319dec6beb57da895042a0918db Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Wed, 14 Oct 2026 22:16:52 +0200 Subject: [PATCH] chore: bump reva to latest main --- go.mod | 2 +- go.sum | 8 +-- .../pkg/storage/fs/posix/tree/assimilation.go | 67 ++++++++++++++++--- .../fs/posix/tree/assimilation/failures.go | 42 +++++++++++- .../metadata/messagepack_backend.go | 12 ++-- vendor/modules.txt | 2 +- 6 files changed, 109 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index e23d26b956..48da754ba1 100644 --- a/go.mod +++ b/go.mod @@ -67,7 +67,7 @@ require ( github.com/open-policy-agent/opa v1.21.1 github.com/opencloud-eu/icap-client v0.0.0-20250930132611-28a2afe62d89 github.com/opencloud-eu/libre-graph-api-go v1.0.9-0.20261008090127-e04fc3d3e458 - github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014095958-0679087922fa + github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014123116-8ee5e6d16f73 github.com/opensearch-project/opensearch-go/v4 v4.7.3 github.com/orcaman/concurrent-map v1.0.0 github.com/pkg/errors v0.9.1 diff --git a/go.sum b/go.sum index 97858d8bcb..0f54b68c76 100644 --- a/go.sum +++ b/go.sum @@ -761,12 +761,8 @@ github.com/opencloud-eu/icap-client v0.0.0-20250930132611-28a2afe62d89 h1:W1ms+l github.com/opencloud-eu/icap-client v0.0.0-20250930132611-28a2afe62d89/go.mod h1:vigJkNss1N2QEceCuNw/ullDehncuJNFB6mEnzfq9UI= github.com/opencloud-eu/libre-graph-api-go v1.0.9-0.20261008090127-e04fc3d3e458 h1:TkJT8H30iesStrBZFHhdwam2rJQA7CoOUwdpbSzxtdk= github.com/opencloud-eu/libre-graph-api-go v1.0.9-0.20261008090127-e04fc3d3e458/go.mod h1:lTM8JeGblNpoMySTW7Lui2+c5TTLI95mwxtdUIHHrhU= -github.com/opencloud-eu/reva/v2 v2.51.1-0.20261013142211-905dc7a89649 h1:7yTv1wY/1oIYh48yQEFRavFzRqj6C9TPksXua34qTug= -github.com/opencloud-eu/reva/v2 v2.51.1-0.20261013142211-905dc7a89649/go.mod h1:rAwuI3PX8n3RxBXHfpMtAKqfwxTIKhZol1Bi3Qj39p8= -github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014072417-be32dce83442 h1:O7atTisd/px+EfXMHCF3t/xigagbh83rrZ+A0jxAar4= -github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014072417-be32dce83442/go.mod h1:rAwuI3PX8n3RxBXHfpMtAKqfwxTIKhZol1Bi3Qj39p8= -github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014095958-0679087922fa h1:ZBOeqAfNvV9TsX/EpL7juhzYwGNgeaw8I494Vm/BGyc= -github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014095958-0679087922fa/go.mod h1:rAwuI3PX8n3RxBXHfpMtAKqfwxTIKhZol1Bi3Qj39p8= +github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014123116-8ee5e6d16f73 h1:6OCfZ2/IROVOEjKNEbhxdrKw1nOIJKYuiODmxuz1sds= +github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014123116-8ee5e6d16f73/go.mod h1:rAwuI3PX8n3RxBXHfpMtAKqfwxTIKhZol1Bi3Qj39p8= github.com/opencloud-eu/secure v0.0.0-20260312082735-b6f5cb2244e4 h1:l2oB/RctH+t8r7QBj5p8thfEHCM/jF35aAY3WQ3hADI= github.com/opencloud-eu/secure v0.0.0-20260312082735-b6f5cb2244e4/go.mod h1:BmF5hyM6tXczk3MpQkFf1hpKSRqCyhqcbiQtiAF7+40= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation.go index 8ce956506e..0db4e62a02 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation.go @@ -439,7 +439,7 @@ func (t *Tree) assimilate(item scanItem) error { t.log.Debug().Str("path", item.Path).Bool("recurse", item.Recurse).Msg("assimilate") var err error - spaceID, id, parentID, mtime, err := t.lookup.MetadataBackend().IdentifyPath(context.Background(), item.Path) + spaceID, id, _, _, err := t.lookup.MetadataBackend().IdentifyPath(context.Background(), item.Path) if err != nil { return err } @@ -487,14 +487,26 @@ func (t *Tree) assimilate(item scanItem) error { _ = unlock() }() + // read the metadata and the file again now that we hold the lock. The previous holder may have changed + // both, e.g. an upload sets the mtime on disk before it writes the mtime attribute. + _, _, parentID, mtime, err := t.lookup.MetadataBackend().IdentifyPath(context.Background(), item.Path) + if err != nil { + return err + } + fi, err = os.Lstat(item.Path) + if err != nil { + return err + } + previousPath, err := t.lookup.GetCachedID(context.Background(), spaceID, id) if previousPath == "" || err != nil { previousPath, err = t.lookup.IDHistoryCache.Get(context.Background(), spaceID, id) } // compare metadata mtime with actual mtime. if it matches AND the path hasn't changed (move operation) - // we can skip the assimilation because the file was handled by us - if previousPath == item.Path && mtime.Equal(fi.ModTime()) { + // we can skip the assimilation because the file was handled by us. A stored name or parent that differs from + // the disk means a rename or a move was missed, even if the id cache already has the new path. + if previousPath == item.Path && mtime.Equal(fi.ModTime()) && !t.metadataIsStale(item.Path) { return _errSkipAlreadyKnown } @@ -975,7 +987,7 @@ func (t *Tree) WarmupIDCache(root string, assimilate, onlyDirty bool) error { sizes[path] += 0 // Make sure to set the size to 0 for empty directories } - nodeSpaceID, id, _, _, err := t.lookup.MetadataBackend().IdentifyPath(context.Background(), path) + nodeSpaceID, id, parentID, mtime, err := t.lookup.MetadataBackend().IdentifyPath(context.Background(), path) if err == nil && len(id) > 0 { if len(nodeSpaceID) > 0 { spaceID = nodeSpaceID @@ -1015,6 +1027,7 @@ func (t *Tree) WarmupIDCache(root string, assimilate, onlyDirty bool) error { if id != "" { // Check if the item on the previous path still exists. In this case it might have been a copy with extended attributes -> set new ID isCopy := false + isMove := false previousPath, err := t.lookup.GetCachedID(context.Background(), spaceID, id) switch err.(type) { case errtypes.NotFound: @@ -1022,18 +1035,38 @@ func (t *Tree) WarmupIDCache(root string, assimilate, onlyDirty bool) error { case nil: if previousPath != path { _, err := os.Stat(previousPath) - if err == nil { + switch { + case err == nil: // previous path (using the same id) still exists -> this is a copy isCopy = true + case os.IsNotExist(err): + // the item is gone from the previous path. It needs to be assimilated + // again only when its own name or parent changed. When an ancestor was + // renamed the item itself stayed as it was and the stale cache entry + // is all there is to fix. + // The walk visits a directory before its children, so a parent created since the + // last scan already has its id here. + _, parentOnDisk, _, _, perr := t.lookup.MetadataBackend().IdentifyPath(context.Background(), filepath.Dir(path)) + isMove = filepath.Base(previousPath) != filepath.Base(path) || + (perr == nil && len(parentOnDisk) > 0 && len(parentID) > 0 && parentID != parentOnDisk) } } default: return errors.Wrap(err, "failed to get previous path from cache") } - if isCopy { - // copy detected -> re-assimilate - _ = t.assimilate(scanItem{Path: path}) + // the metadata mtime lags behind the one on disk when a file changed outside of + // opencloud. Directories are left out because their mtime on disk also changes + // when a child is added or removed, and the walk visits those children anyway. + isChanged := !info.IsDir() && !mtime.IsZero() && !mtime.Equal(info.ModTime()) + + if isCopy || (assimilate && (isMove || isChanged || t.metadataIsStale(path))) { + // a copy with a clashing id, a move or a change on disk -> re-assimilate. + // assimilate skips items whose path and mtime still match the metadata + err := t.assimilate(scanItem{Path: path}) + if err != nil && !errors.Is(err, _errSkipAlreadyKnown) { + t.log.Error().Err(err).Str("path", path).Msg("could not assimilate item") + } } else { // update cached id with new path if err := t.lookup.CacheID(context.Background(), spaceID, id, path); err != nil { @@ -1085,6 +1118,24 @@ func (t *Tree) WarmupIDCache(root string, assimilate, onlyDirty bool) error { return nil } +// metadataIsStale reports whether the name or the parent id stored for an item differ from where the item is on +// disk. It reads the attributes from disk, like IdentifyPath, so a cached copy can't hide or fake a difference. +// Space roots are left out, because their name doesn't have to match the name of their directory. +func (t *Tree) metadataIsStale(path string) bool { + if t.Ignorer.IsSpaceRoot(path) { + return false + } + if name, err := xattr.Get(path, prefixes.NameAttr); err == nil && string(name) != filepath.Base(path) { + return true + } + parentID, err := xattr.Get(path, prefixes.ParentidAttr) + if err != nil || len(parentID) == 0 { + return false + } + parentOnDisk, err := xattr.Get(filepath.Dir(path), prefixes.IDAttr) + return err == nil && len(parentOnDisk) > 0 && string(parentID) != string(parentOnDisk) +} + func (t *Tree) propagateSizeDiff(n *node.Node, size int64) error { attrs, err := t.lookup.MetadataBackend().All(context.Background(), n) if err != nil { diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation/failures.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation/failures.go index 9cb536a908..501bba84f1 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation/failures.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/tree/assimilation/failures.go @@ -3,11 +3,14 @@ package assimilation import ( "io/fs" + "os" "syscall" "time" "github.com/hashicorp/golang-lru/v2/expirable" "github.com/pkg/errors" + + "github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/timemanager" ) const ( @@ -28,6 +31,7 @@ type Failures struct { type failure struct { err error modTime time.Time + cTime time.Time size int64 mode fs.FileMode uid uint32 @@ -63,14 +67,27 @@ func (f *Failures) Record(path string, fi fs.FileInfo, err error) { return } + // keep the doubled delay when only the ctime changed, so a file that gets fixed again and + // again costs one read per fix instead of starting over at the minimum delay delay := minRetryDelay - if last, ok := f.lru.Peek(path); ok && last.unchanged(fi) { + if last, ok := f.lru.Peek(path); ok && last.sameAttributes(fi) { delay = min(2*last.delay, maxRetryDelay) } + + // every attribute the attempt managed to write changed the ctime, so read it again here. A + // change after this point, for example a setfacl or a chattr -i, then triggers a retry. + // Without a fresh stat the ctime stays unknown, which is better than keeping the one from + // before the attempt: that one never matches again, so every scan would read the file. + ctime := time.Time{} + if after, err := os.Lstat(path); err == nil { + ctime = cTime(after) + } + uid, gid := owner(fi) f.lru.Add(path, failure{ err: err, modTime: fi.ModTime(), + cTime: ctime, size: fi.Size(), mode: fi.Mode(), uid: uid, @@ -80,13 +97,32 @@ func (f *Failures) Record(path string, fi fs.FileInfo, err error) { }) } -// unchanged reports whether fi still matches the file that failed. It includes the owner, so a chown -// that fixes the file triggers a retry. +// unchanged reports whether fi still matches the file that failed. A zero ctime means it is +// unknown, and only the attributes decide. func (f failure) unchanged(fi fs.FileInfo) bool { + return f.sameAttributes(fi) && (f.cTime.IsZero() || f.cTime.Equal(cTime(fi))) +} + +// sameAttributes reports whether the attributes of fi still match the file that failed. It includes +// the owner, so a chown that fixes the file triggers a retry. A setfacl or a chattr changes none of +// them, which is why unchanged looks at the ctime as well. +func (f failure) sameAttributes(fi fs.FileInfo) bool { uid, gid := owner(fi) return f.modTime.Equal(fi.ModTime()) && f.size == fi.Size() && f.mode == fi.Mode() && f.uid == uid && f.gid == gid } +// cTime returns the time of the last change to the file or its attributes. It is zero on systems +// that don't report one, which leaves unchanged to the attributes above. +func cTime(fi fs.FileInfo) time.Time { + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return time.Time{} + } + ts := timemanager.StatCTime(st) + //nolint:unconvert + return time.Unix(int64(ts.Sec), int64(ts.Nsec)) +} + func owner(fi fs.FileInfo) (uint32, uint32) { if st, ok := fi.Sys().(*syscall.Stat_t); ok { return st.Uid, st.Gid diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go index 1286062afb..4a38300965 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go @@ -250,11 +250,13 @@ func (b MessagePackBackend) loadAttributes(ctx context.Context, path string, sou if err != nil { return nil, err } - if len(msgBytes) > 0 { - err = msgpack.Unmarshal(msgBytes, &attribs) - if err != nil { - return nil, err - } + if len(msgBytes) == 0 { + return nil, errors.New("encountered empty metadata file") + } + + err = msgpack.Unmarshal(msgBytes, &attribs) + if err != nil { + return nil, err } _, subspan := tracer.Start(ctx, "metaCache.PushToCache") diff --git a/vendor/modules.txt b/vendor/modules.txt index a757596355..85d749e781 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1360,7 +1360,7 @@ github.com/opencloud-eu/icap-client # github.com/opencloud-eu/libre-graph-api-go v1.0.9-0.20261008090127-e04fc3d3e458 ## explicit; go 1.23 github.com/opencloud-eu/libre-graph-api-go -# github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014095958-0679087922fa +# github.com/opencloud-eu/reva/v2 v2.51.1-0.20261014123116-8ee5e6d16f73 ## explicit; go 1.26.0 github.com/opencloud-eu/reva/v2/cmd/revad/internal/grace github.com/opencloud-eu/reva/v2/cmd/revad/runtime