From e2a37f118904927a7d79aba223f3b6498845899a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 25 Apr 2023 14:20:28 +0200 Subject: [PATCH] filemetadata: List less cache invalidation (#3812) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * filemetadata: List less cache invalidation Signed-off-by: Jörn Friedrich Dreyer * Use the proper cache key when invalidating early * Consistently use original path as the cache key * Fix renaming entries in the cache --------- Signed-off-by: Jörn Friedrich Dreyer Co-authored-by: André Duffeck --- .../list-less-cache-invalidation.md | 3 ++ pkg/storage/cache/filemetadata.go | 13 +------ .../metadata/messagepack_backend.go | 36 ++++++++----------- 3 files changed, 19 insertions(+), 33 deletions(-) create mode 100644 changelog/unreleased/list-less-cache-invalidation.md diff --git a/changelog/unreleased/list-less-cache-invalidation.md b/changelog/unreleased/list-less-cache-invalidation.md new file mode 100644 index 0000000000..d862a7b1f2 --- /dev/null +++ b/changelog/unreleased/list-less-cache-invalidation.md @@ -0,0 +1,3 @@ +Bugfix: Filemetadata Cache now deletes keys without listing them first + +https://github.com/cs3org/reva/pull/3812 diff --git a/pkg/storage/cache/filemetadata.go b/pkg/storage/cache/filemetadata.go index 58be251881..9f8828cccc 100644 --- a/pkg/storage/cache/filemetadata.go +++ b/pkg/storage/cache/filemetadata.go @@ -19,7 +19,6 @@ package cache import ( - "strings" "time" ) @@ -40,15 +39,5 @@ func NewFileMetadataCache(store string, nodes []string, database, table string, // RemoveMetadata removes a reference from the metadata cache func (c *fileMetadataCache) RemoveMetadata(path string) error { - keys, err := c.List() - if err != nil { - return err - } - - for _, key := range keys { - if strings.HasPrefix(key, path) { - _ = c.Delete(key) - } - } - return nil + return c.s.Delete(path) } diff --git a/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go b/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go index b414ede94e..4152e56fb6 100644 --- a/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go +++ b/pkg/storage/utils/decomposedfs/metadata/messagepack_backend.go @@ -57,15 +57,11 @@ func (MessagePackBackend) Name() string { return "messagepack" } // All reads all extended attributes for a node func (b MessagePackBackend) All(path string) (map[string][]byte, error) { - path = b.MetadataPath(path) - return b.loadAttributes(path, nil) } // Get an extended attribute value for the given key func (b MessagePackBackend) Get(path, key string) ([]byte, error) { - path = b.MetadataPath(path) - attribs, err := b.loadAttributes(path, nil) if err != nil { return []byte{}, err @@ -79,8 +75,6 @@ func (b MessagePackBackend) Get(path, key string) ([]byte, error) { // GetInt64 reads a string as int64 from the xattrs func (b MessagePackBackend) GetInt64(path, key string) (int64, error) { - path = b.MetadataPath(path) - attribs, err := b.loadAttributes(path, nil) if err != nil { return 0, err @@ -99,8 +93,6 @@ func (b MessagePackBackend) GetInt64(path, key string) (int64, error) { // List retrieves a list of names of extended attributes associated with the // given path in the file system. func (b MessagePackBackend) List(path string) ([]string, error) { - path = b.MetadataPath(path) - attribs, err := b.loadAttributes(path, nil) if err != nil { return nil, err @@ -130,7 +122,6 @@ func (b MessagePackBackend) Remove(path, key string) error { // AllWithLockedSource reads all extended attributes from the given reader (if possible). // The path argument is used for storing the data in the cache func (b MessagePackBackend) AllWithLockedSource(path string, source io.Reader) (map[string][]byte, error) { - path = b.MetadataPath(path) return b.loadAttributes(path, source) } @@ -139,11 +130,11 @@ func (b MessagePackBackend) saveAttributes(path string, setAttribs map[string][] f readWriteCloseSeekTruncater err error ) - path = b.MetadataPath(path) + metaPath := b.MetadataPath(path) if acquireLock { - f, err = lockedfile.OpenFile(path, os.O_RDWR|os.O_CREATE, 0600) + f, err = lockedfile.OpenFile(metaPath, os.O_RDWR|os.O_CREATE, 0600) } else { - f, err = os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0600) + f, err = os.OpenFile(metaPath, os.O_RDWR|os.O_CREATE, 0600) } if err != nil { return err @@ -151,7 +142,7 @@ func (b MessagePackBackend) saveAttributes(path string, setAttribs map[string][] defer f.Close() // Invalidate cache early - _ = b.metaCache.RemoveMetadata(path) + _ = b.metaCache.RemoveMetadata(b.cacheKey(path)) // Read current state msgBytes, err := io.ReadAll(f) @@ -204,15 +195,16 @@ func (b MessagePackBackend) loadAttributes(path string, source io.Reader) (map[s return attribs, err } + metaPath := b.MetadataPath(path) if source == nil { - source, err = lockedfile.Open(path) + source, err = lockedfile.Open(metaPath) // // No cached entry found. Read from storage and store in cache if err != nil { if os.IsNotExist(err) { // some of the caller rely on ENOTEXISTS to be returned when the // actual file (not the metafile) does not exist in order to // determine whether a node exists or not -> stat the actual node - _, err := os.Stat(strings.TrimSuffix(path, ".mpk")) + _, err := os.Stat(path) if err != nil { return nil, err } @@ -254,13 +246,15 @@ func (b MessagePackBackend) Purge(path string) error { // Rename moves the data for a given path to a new path func (b MessagePackBackend) Rename(oldPath, newPath string) error { - data := map[string]string{} - _ = b.metaCache.PullFromCache(b.cacheKey(oldPath), &data) - err := b.metaCache.RemoveMetadata(b.cacheKey(oldPath)) - if err != nil { - return err + data := map[string][]byte{} + err := b.metaCache.PullFromCache(b.cacheKey(oldPath), &data) + if err == nil { + err = b.metaCache.PushToCache(b.cacheKey(newPath), data) + if err != nil { + return err + } } - err = b.metaCache.PushToCache(b.cacheKey(newPath), data) + err = b.metaCache.RemoveMetadata(b.cacheKey(oldPath)) if err != nil { return err }