Skip to content

Commit e8570ca

Browse files
fix: retry getting xattrs on concurrent write error (#700)
1 parent 876aad3 commit e8570ca

2 files changed

Lines changed: 34 additions & 0 deletions

File tree

‎pkg/storage/utils/decomposedfs/metadata/errors.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@ func IsNotExist(err error) bool {
4545
return false
4646
}
4747

48+
// IsErrRange checks for a syscall.ERANGE buried inside an xattr error. listxattr
49+
// returns ERANGE when the set of attribute names grows between the size-probe call
50+
// and the read call (a concurrent setxattr landed in between). This is transient and
51+
// self-clearing once the competing writer finishes its batch.
52+
func IsErrRange(err error) bool {
53+
if xerr, ok := errors.Cause(err).(*xattr.Error); ok {
54+
if serr, ok2 := xerr.Err.(syscall.Errno); ok2 {
55+
return serr == syscall.ERANGE
56+
}
57+
}
58+
return false
59+
}
60+
4861
// IsAttrUnset checks the xattr.ENOATTR from the xattr package which redifines it as ENODATA on platforms that do not natively support it (eg. linux)
4962
// see https://github.com/pkg/xattr/blob/8725d4ccc0fcef59c8d9f0eaf606b3c6f962467a/xattr_linux.go#L19-L22
5063
func IsAttrUnset(err error) bool {

‎pkg/storage/utils/decomposedfs/metadata/xattrs_backend.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"strconv"
2727
"strings"
2828

29+
"github.com/owncloud/reva/v2/pkg/appctx"
2930
"github.com/owncloud/reva/v2/pkg/storage/cache"
3031
"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/metadata/prefixes"
3132
"github.com/owncloud/reva/v2/pkg/storage/utils/filelocks"
@@ -82,12 +83,32 @@ func (b XattrsBackend) List(ctx context.Context, filePath string) (attribs []str
8283
return b.list(ctx, filePath, true)
8384
}
8485

86+
const listMaxRetries = 10
87+
8588
func (b XattrsBackend) list(ctx context.Context, filePath string, acquireLock bool) (attribs []string, err error) {
8689
attrs, err := xattr.List(filePath)
8790
if err == nil {
8891
return attrs, nil
8992
}
9093

94+
// ERANGE means a concurrent writer (e.g. the posix inotify assimilate worker)
95+
// mutated the attribute set mid-read. It is transient and self-clearing: retry
96+
// until the set stabilises.
97+
if IsErrRange(err) {
98+
for attempt := 1; attempt <= listMaxRetries; attempt++ {
99+
appctx.GetLogger(ctx).Debug().Err(err).Str("path", filePath).Int("attempt", attempt).Msg("xattr.List ERANGE, retrying")
100+
attrs, err = xattr.List(filePath)
101+
if err == nil {
102+
appctx.GetLogger(ctx).Debug().Str("path", filePath).Int("attempt", attempt).Msg("xattr.List ERANGE recovered after retry")
103+
return attrs, nil
104+
}
105+
if !IsErrRange(err) {
106+
break
107+
}
108+
}
109+
appctx.GetLogger(ctx).Error().Err(err).Str("path", filePath).Msg("xattr.List still failing after ERANGE retries")
110+
}
111+
91112
// listing xattrs failed, try again, either with lock or without
92113
if acquireLock {
93114
f, err := lockedfile.OpenFile(filePath+filelocks.LockFileSuffix, os.O_CREATE|os.O_WRONLY, 0600)

0 commit comments

Comments
 (0)