diff --git a/changelog/unreleased/reduce-lock-contention.md b/changelog/unreleased/reduce-lock-contention.md new file mode 100644 index 0000000000..6e2fe32472 --- /dev/null +++ b/changelog/unreleased/reduce-lock-contention.md @@ -0,0 +1,6 @@ +Enhancement: Reduce lock contention issues + +We reduced lock contention during high load by optimistically non-locking when listing the extended attributes of a file. +Only in case of issues the list is read again while holding a lock. + +https://github.com/cs3org/reva/pull/3395 diff --git a/pkg/storage/utils/decomposedfs/node/locks.go b/pkg/storage/utils/decomposedfs/node/locks.go index 15957cb67d..16dd222681 100644 --- a/pkg/storage/utils/decomposedfs/node/locks.go +++ b/pkg/storage/utils/decomposedfs/node/locks.go @@ -255,20 +255,20 @@ func (n *Node) Unlock(ctx context.Context, lock *provider.Lock) error { // CheckLock compares the context lock with the node lock func (n *Node) CheckLock(ctx context.Context) error { - lockID, _ := ctxpkg.ContextGetLockID(ctx) - lock, _ := n.ReadLock(ctx, false) - if lock != nil { - switch lockID { + contextLock, _ := ctxpkg.ContextGetLockID(ctx) + diskLock, _ := n.ReadLock(ctx, false) + if diskLock != nil { + switch contextLock { case "": - return errtypes.Locked(lock.LockId) // no lockid in request - case lock.LockId: + return errtypes.Locked(diskLock.LockId) // no lockid in request + case diskLock.LockId: return nil // ok default: return errtypes.Aborted("mismatching lock") } } - if lockID != "" { - return errtypes.Aborted("not locked") + if contextLock != "" { + return errtypes.Aborted("not locked") // no lock on disk. why is there a lockid in the context } return nil // ok } diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index 6ae8a8b4cf..732e8e880c 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -189,8 +189,6 @@ func CopyMetadata(src, target string, filter func(attributeName string) bool) (e } // Set an extended attribute key to the given value -// No file locking is involved here as writing a single xattr is -// considered to be atomic. func Set(filePath string, key string, val string) (err error) { fileLock, err := filelocks.AcquireWriteLock(filePath) @@ -210,8 +208,6 @@ func Set(filePath string, key string, val string) (err error) { } // Remove an extended attribute key -// No file locking is involved here as writing a single xattr is -// considered to be atomic. func Remove(filePath string, key string) (err error) { fileLock, err := filelocks.AcquireWriteLock(filePath) @@ -235,9 +231,6 @@ func Remove(filePath string, key string) (err error) { // If the file lock can not be acquired the function returns a // lock error. func SetMultiple(filePath string, attribs map[string]string) (err error) { - - // h, err := lockedfile.OpenFile(filePath, os.O_WRONLY, 0) // 0? Open File only workn for files ... but we want to lock dirs ... or symlinks - // or we append .lock to the file and use https://github.com/gofrs/flock var fileLock *flock.Flock fileLock, err = filelocks.AcquireWriteLock(filePath) @@ -299,7 +292,12 @@ func GetInt64(filePath, key string) (int64, error) { // List retrieves a list of names of extended attributes associated with the // given path in the file system. func List(filePath string) (attribs []string, err error) { - // now try to get a shared lock on the source + attrs, err := xattr.List(filePath) + if err == nil { + return attrs, nil + } + + // listing the attributes failed. lock the file and try again readLock, err := filelocks.AcquireReadLock(filePath) if err != nil {