From b8bc1b5b1c2c2652b6fc85209cc178017fc11bd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 25 Nov 2022 10:02:01 +0000 Subject: [PATCH 1/3] decomposedfs: configurable filelock duration factor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/filelock-duration-factor.md | 5 +++ .../utils/decomposedfs/decomposedfs.go | 4 +++ .../utils/decomposedfs/options/options.go | 3 +- pkg/storage/utils/filelocks/filelocks.go | 20 ++++++++--- pkg/storage/utils/filelocks/filelocks_test.go | 34 +++++++++++++++++++ 5 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 changelog/unreleased/filelock-duration-factor.md 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..1c9e95deb6 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 durig 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..be3930801c 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,38 @@ func TestAcquireReadLock(t *testing.T) { wg.Wait() } +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() From 558f6fedaad7be52a6217abbe37921b6d4e05ce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 25 Nov 2022 12:11:12 +0000 Subject: [PATCH 2/3] disable flaky negative test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/utils/filelocks/filelocks_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/storage/utils/filelocks/filelocks_test.go b/pkg/storage/utils/filelocks/filelocks_test.go index be3930801c..b6cf8ab436 100644 --- a/pkg/storage/utils/filelocks/filelocks_test.go +++ b/pkg/storage/utils/filelocks/filelocks_test.go @@ -88,6 +88,7 @@ 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() @@ -119,6 +120,7 @@ func TestAcquireReadLockFail(t *testing.T) { wg.Wait() } +*/ func TestReleaseLock(t *testing.T) { file, fin, _ := filelocks.FileFactory() From 382abe197718d3b06eaed1e4b0d2f26fe4a8542a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 25 Nov 2022 12:21:04 +0000 Subject: [PATCH 3/3] fix typos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/utils/filelocks/filelocks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/utils/filelocks/filelocks.go b/pkg/storage/utils/filelocks/filelocks.go index 1c9e95deb6..79e91332ea 100644 --- a/pkg/storage/utils/filelocks/filelocks.go +++ b/pkg/storage/utils/filelocks/filelocks.go @@ -52,7 +52,7 @@ func SetMaxLockCycles(v int) { }) } -// SetLockCycleDurationFactor configures the factor applied to the timeout allowed durig a lock cycle. Subsequent calls to SetLockCycleDurationFactor have no effect +// 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