filemetadata: List less cache invalidation (#3812)

* filemetadata: List less cache invalidation

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

* 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 <[email protected]>
Co-authored-by: André Duffeck <[email protected]>

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

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)
 }

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,19 +130,19 @@ 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
 	}
 	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
 	}