diff --git a/changelog/unreleased/filelock-duration-factor.md b/changelog/unreleased/filelock-duration-factor.md new file mode 100644 index 0000000000..bd282d42de --- /dev/null +++ b/changelog/unreleased/filelock-duration-factor.md @@ -0,0 +1,5 @@ +Enhancement: configurable filelock duration factor in decomposedfs + +The lock cycle duration factor in decomposedfs can now be changed by setting `lock_cycle_duration_factor`. + +https://github.com/cs3org/reva/pull/3493 diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index ccfb276176..354c5c20df 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -137,6 +137,10 @@ func New(o *options.Options, lu *lookup.Lookup, p PermissionsChecker, tp Tree, p filelocks.SetMaxLockCycles(o.MaxAcquireLockCycles) } + if o.LockCycleDurationFactor != 0 { + filelocks.SetLockCycleDurationFactor(o.LockCycleDurationFactor) + } + return &Decomposedfs{ tp: tp, lu: lu, diff --git a/pkg/storage/utils/decomposedfs/options/options.go b/pkg/storage/utils/decomposedfs/options/options.go index 705f9c8638..3ae5b5cbdf 100644 --- a/pkg/storage/utils/decomposedfs/options/options.go +++ b/pkg/storage/utils/decomposedfs/options/options.go @@ -53,7 +53,8 @@ type Options struct { PersonalSpaceAliasTemplate string `mapstructure:"personalspacealias_template"` GeneralSpaceAliasTemplate string `mapstructure:"generalspacealias_template"` - MaxAcquireLockCycles int `mapstructure:"max_acquire_lock_cycles"` + MaxAcquireLockCycles int `mapstructure:"max_acquire_lock_cycles"` + LockCycleDurationFactor int `mapstructure:"lock_cycle_duration_factor"` } // New returns a new Options instance for the given configuration diff --git a/pkg/storage/utils/filelocks/filelocks.go b/pkg/storage/utils/filelocks/filelocks.go index 8816cbf1b6..79e91332ea 100644 --- a/pkg/storage/utils/filelocks/filelocks.go +++ b/pkg/storage/utils/filelocks/filelocks.go @@ -32,9 +32,12 @@ import ( const LockFileSuffix = ".flock" var ( - _localLocks sync.Map - _lockCycles sync.Once - _lockCyclesValue = 25 + _localLocks sync.Map + // waiting 20 lock cycles with a factor of 30 yields 6300ms, or a little over 6 sec + _lockCycles sync.Once + _lockCyclesValue = 20 + _lockCycleDuration sync.Once + _lockCycleDurationFactor = 30 // ErrPathEmpty indicates that no path was specified ErrPathEmpty = errors.New("lock path is empty") @@ -49,6 +52,13 @@ func SetMaxLockCycles(v int) { }) } +// SetLockCycleDurationFactor configures the factor applied to the timeout allowed during a lock cycle. Subsequent calls to SetLockCycleDurationFactor have no effect +func SetLockCycleDurationFactor(v int) { + _lockCycleDuration.Do(func() { + _lockCycleDurationFactor = v + }) +} + // getMutexedFlock returns a new Flock struct for the given file. // If there is already one in the local store, it returns nil. // The caller has to wait until it can get a new one out of this @@ -93,7 +103,7 @@ func acquireLock(file string, write bool) (*flock.Flock, error) { if flock = getMutexedFlock(n); flock != nil { break } - w := time.Duration(i*3) * time.Millisecond + w := time.Duration(i*_lockCycleDurationFactor) * time.Millisecond time.Sleep(w) } @@ -113,7 +123,7 @@ func acquireLock(file string, write bool) (*flock.Flock, error) { break } - time.Sleep(time.Duration(i*3) * time.Millisecond) + time.Sleep(time.Duration(i*_lockCycleDurationFactor) * time.Millisecond) } if !ok { diff --git a/pkg/storage/utils/filelocks/filelocks_test.go b/pkg/storage/utils/filelocks/filelocks_test.go index 09319f314e..b6cf8ab436 100644 --- a/pkg/storage/utils/filelocks/filelocks_test.go +++ b/pkg/storage/utils/filelocks/filelocks_test.go @@ -32,6 +32,7 @@ func TestAcquireWriteLock(t *testing.T) { defer fin() filelocks.SetMaxLockCycles(90) + filelocks.SetLockCycleDurationFactor(3) var wg sync.WaitGroup for i := 0; i < 10; i++ { @@ -61,6 +62,7 @@ func TestAcquireReadLock(t *testing.T) { defer fin() filelocks.SetMaxLockCycles(90) + filelocks.SetLockCycleDurationFactor(3) var wg sync.WaitGroup for i := 0; i < 10; i++ { @@ -86,6 +88,40 @@ func TestAcquireReadLock(t *testing.T) { wg.Wait() } +/* This negative test is flaky as 8000 goroutines are not enough to trigger this in ci +func TestAcquireReadLockFail(t *testing.T) { + file, fin, _ := filelocks.FileFactory() + defer fin() + + filelocks.SetMaxLockCycles(1) + filelocks.SetLockCycleDurationFactor(1) + + // create a channel big enough for all waiting groups + errors := make(chan error, 8000) + var wg sync.WaitGroup + for i := 0; i < 8000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + + l, err := filelocks.AcquireReadLock(file) + if err != nil { + // collect the error in a channel + errors <- err + return + } + err = filelocks.ReleaseLock(l) + assert.Nil(t, err) + }() + } + + // at least one error should have occurred + assert.NotNil(t, <-errors) + + wg.Wait() +} +*/ + func TestReleaseLock(t *testing.T) { file, fin, _ := filelocks.FileFactory() defer fin()