From ea59896bd61990cfa8b7b31b1c9eb9ebd7000ed4 Mon Sep 17 00:00:00 2001 From: jo Date: Thu, 16 Feb 2023 16:58:36 +0100 Subject: [PATCH] Add a global option --retry-lock Fixes restic#719 If the option is passed, restic will wait the specified duration of time and retry locking the repo every 10 seconds (or more often if the total timeout is relatively small). - Play nice with json output - Reduce wait time in lock tests - Rework timeout last attempt - Reduce test wait time to 0.1s - Use exponential back off for the retry lock - Don't pass gopts to lockRepo functions - Use global variable for retry sleep setup - Exit retry lock on cancel - Better wording for flag help - Reorder debug statement - Refactor tests - Lower max sleep time to 1m - Test that we cancel/timeout in time - Use non blocking sleep function - Refactor into minDuration func Co-authored-by: Julian Brost --- changelog/unreleased/issue-719 | 8 +++ cmd/restic/cmd_backup.go | 2 +- cmd/restic/cmd_cat.go | 2 +- cmd/restic/cmd_check.go | 2 +- cmd/restic/cmd_copy.go | 4 +- cmd/restic/cmd_debug.go | 4 +- cmd/restic/cmd_diff.go | 2 +- cmd/restic/cmd_dump.go | 2 +- cmd/restic/cmd_find.go | 2 +- cmd/restic/cmd_forget.go | 2 +- cmd/restic/cmd_key.go | 8 +-- cmd/restic/cmd_list.go | 8 +-- cmd/restic/cmd_migrate.go | 2 +- cmd/restic/cmd_mount.go | 2 +- cmd/restic/cmd_prune.go | 2 +- cmd/restic/cmd_rebuild_index.go | 2 +- cmd/restic/cmd_recover.go | 2 +- cmd/restic/cmd_restore.go | 2 +- cmd/restic/cmd_rewrite.go | 4 +- cmd/restic/cmd_snapshots.go | 2 +- cmd/restic/cmd_stats.go | 2 +- cmd/restic/cmd_tag.go | 2 +- cmd/restic/global.go | 2 + cmd/restic/lock.go | 61 +++++++++++++++++-- cmd/restic/lock_test.go | 101 +++++++++++++++++++++++++++----- doc/design.rst | 5 +- doc/manual_rest.rst | 2 + 27 files changed, 188 insertions(+), 51 deletions(-) create mode 100644 changelog/unreleased/issue-719 diff --git a/changelog/unreleased/issue-719 b/changelog/unreleased/issue-719 new file mode 100644 index 000000000..4f28ea83c --- /dev/null +++ b/changelog/unreleased/issue-719 @@ -0,0 +1,8 @@ +Enhancement: Add --retry-lock option + +This option allows to specify a duration for which restic will wait if there +already exists a conflicting lock within the repository. + +https://github.com/restic/restic/issues/719 +https://github.com/restic/restic/pull/2214 +https://github.com/restic/restic/pull/4107 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index 1244e2ed1..fcaff304d 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -506,7 +506,7 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter if !gopts.JSON { progressPrinter.V("lock repository") } - lock, ctx, err := lockRepo(ctx, repo) + lock, ctx, err := lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_cat.go b/cmd/restic/cmd_cat.go index e7253e5b6..771731a58 100644 --- a/cmd/restic/cmd_cat.go +++ b/cmd/restic/cmd_cat.go @@ -45,7 +45,7 @@ func runCat(ctx context.Context, gopts GlobalOptions, args []string) error { if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index d56f7d0c9..e5f29a7e5 100644 --- a/cmd/restic/cmd_check.go +++ b/cmd/restic/cmd_check.go @@ -211,7 +211,7 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args if !gopts.NoLock { Verbosef("create exclusive lock for repository\n") var lock *restic.Lock - lock, ctx, err = lockRepoExclusive(ctx, repo) + lock, ctx, err = lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_copy.go b/cmd/restic/cmd_copy.go index 2f095972a..13767d98a 100644 --- a/cmd/restic/cmd_copy.go +++ b/cmd/restic/cmd_copy.go @@ -74,14 +74,14 @@ func runCopy(ctx context.Context, opts CopyOptions, gopts GlobalOptions, args [] if !gopts.NoLock { var srcLock *restic.Lock - srcLock, ctx, err = lockRepo(ctx, srcRepo) + srcLock, ctx, err = lockRepo(ctx, srcRepo, gopts.RetryLock, gopts.JSON) defer unlockRepo(srcLock) if err != nil { return err } } - dstLock, ctx, err := lockRepo(ctx, dstRepo) + dstLock, ctx, err := lockRepo(ctx, dstRepo, gopts.RetryLock, gopts.JSON) defer unlockRepo(dstLock) if err != nil { return err diff --git a/cmd/restic/cmd_debug.go b/cmd/restic/cmd_debug.go index c8626d46c..deade6d22 100644 --- a/cmd/restic/cmd_debug.go +++ b/cmd/restic/cmd_debug.go @@ -156,7 +156,7 @@ func runDebugDump(ctx context.Context, gopts GlobalOptions, args []string) error if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err @@ -462,7 +462,7 @@ func runDebugExamine(ctx context.Context, gopts GlobalOptions, args []string) er if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_diff.go b/cmd/restic/cmd_diff.go index 0000fd18a..0861a7103 100644 --- a/cmd/restic/cmd_diff.go +++ b/cmd/restic/cmd_diff.go @@ -334,7 +334,7 @@ func runDiff(ctx context.Context, opts DiffOptions, gopts GlobalOptions, args [] if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index cda7b65b9..34313f582 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -132,7 +132,7 @@ func runDump(ctx context.Context, opts DumpOptions, gopts GlobalOptions, args [] if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_find.go b/cmd/restic/cmd_find.go index e5457c3be..3ef5f26bf 100644 --- a/cmd/restic/cmd_find.go +++ b/cmd/restic/cmd_find.go @@ -575,7 +575,7 @@ func runFind(ctx context.Context, opts FindOptions, gopts GlobalOptions, args [] if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_forget.go b/cmd/restic/cmd_forget.go index fbe4c1c8a..b958a81db 100644 --- a/cmd/restic/cmd_forget.go +++ b/cmd/restic/cmd_forget.go @@ -116,7 +116,7 @@ func runForget(ctx context.Context, opts ForgetOptions, gopts GlobalOptions, arg if !opts.DryRun || !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepoExclusive(ctx, repo) + lock, ctx, err = lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_key.go b/cmd/restic/cmd_key.go index 88b6d5c0c..62521d762 100644 --- a/cmd/restic/cmd_key.go +++ b/cmd/restic/cmd_key.go @@ -212,7 +212,7 @@ func runKey(ctx context.Context, gopts GlobalOptions, args []string) error { switch args[0] { case "list": - lock, ctx, err := lockRepo(ctx, repo) + lock, ctx, err := lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err @@ -220,7 +220,7 @@ func runKey(ctx context.Context, gopts GlobalOptions, args []string) error { return listKeys(ctx, repo, gopts) case "add": - lock, ctx, err := lockRepo(ctx, repo) + lock, ctx, err := lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err @@ -228,7 +228,7 @@ func runKey(ctx context.Context, gopts GlobalOptions, args []string) error { return addKey(ctx, repo, gopts) case "remove": - lock, ctx, err := lockRepoExclusive(ctx, repo) + lock, ctx, err := lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err @@ -241,7 +241,7 @@ func runKey(ctx context.Context, gopts GlobalOptions, args []string) error { return deleteKey(ctx, repo, id) case "passwd": - lock, ctx, err := lockRepoExclusive(ctx, repo) + lock, ctx, err := lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_list.go b/cmd/restic/cmd_list.go index 4809092c0..bd02cedc7 100644 --- a/cmd/restic/cmd_list.go +++ b/cmd/restic/cmd_list.go @@ -31,19 +31,19 @@ func init() { cmdRoot.AddCommand(cmdList) } -func runList(ctx context.Context, cmd *cobra.Command, opts GlobalOptions, args []string) error { +func runList(ctx context.Context, cmd *cobra.Command, gopts GlobalOptions, args []string) error { if len(args) != 1 { return errors.Fatal("type not specified, usage: " + cmd.Use) } - repo, err := OpenRepository(ctx, opts) + repo, err := OpenRepository(ctx, gopts) if err != nil { return err } - if !opts.NoLock && args[0] != "locks" { + if !gopts.NoLock && args[0] != "locks" { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_migrate.go b/cmd/restic/cmd_migrate.go index 6d614be39..fd2e762c0 100644 --- a/cmd/restic/cmd_migrate.go +++ b/cmd/restic/cmd_migrate.go @@ -122,7 +122,7 @@ func runMigrate(ctx context.Context, opts MigrateOptions, gopts GlobalOptions, a return err } - lock, ctx, err := lockRepoExclusive(ctx, repo) + lock, ctx, err := lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_mount.go b/cmd/restic/cmd_mount.go index 0501bfe89..ec3662d5c 100644 --- a/cmd/restic/cmd_mount.go +++ b/cmd/restic/cmd_mount.go @@ -123,7 +123,7 @@ func runMount(ctx context.Context, opts MountOptions, gopts GlobalOptions, args if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_prune.go b/cmd/restic/cmd_prune.go index f59be2967..6104002b0 100644 --- a/cmd/restic/cmd_prune.go +++ b/cmd/restic/cmd_prune.go @@ -167,7 +167,7 @@ func runPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions) error opts.unsafeRecovery = true } - lock, ctx, err := lockRepoExclusive(ctx, repo) + lock, ctx, err := lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_rebuild_index.go b/cmd/restic/cmd_rebuild_index.go index 6d49cb917..5d70a9e12 100644 --- a/cmd/restic/cmd_rebuild_index.go +++ b/cmd/restic/cmd_rebuild_index.go @@ -49,7 +49,7 @@ func runRebuildIndex(ctx context.Context, opts RebuildIndexOptions, gopts Global return err } - lock, ctx, err := lockRepoExclusive(ctx, repo) + lock, ctx, err := lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index 65f4c8750..85dcc23d7 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -46,7 +46,7 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error { return err } - lock, ctx, err := lockRepo(ctx, repo) + lock, ctx, err := lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index a8b4f8069..a3e602c8f 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -154,7 +154,7 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_rewrite.go b/cmd/restic/cmd_rewrite.go index 0d9aa1c8c..8a1b860ed 100644 --- a/cmd/restic/cmd_rewrite.go +++ b/cmd/restic/cmd_rewrite.go @@ -164,9 +164,9 @@ func runRewrite(ctx context.Context, opts RewriteOptions, gopts GlobalOptions, a var err error if opts.Forget { Verbosef("create exclusive lock for repository\n") - lock, ctx, err = lockRepoExclusive(ctx, repo) + lock, ctx, err = lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) } else { - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) } defer unlockRepo(lock) if err != nil { diff --git a/cmd/restic/cmd_snapshots.go b/cmd/restic/cmd_snapshots.go index 2de8801cb..ba3644ee7 100644 --- a/cmd/restic/cmd_snapshots.go +++ b/cmd/restic/cmd_snapshots.go @@ -65,7 +65,7 @@ func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts GlobalOptions if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_stats.go b/cmd/restic/cmd_stats.go index 55ba6f254..e8558d290 100644 --- a/cmd/restic/cmd_stats.go +++ b/cmd/restic/cmd_stats.go @@ -83,7 +83,7 @@ func runStats(ctx context.Context, gopts GlobalOptions, args []string) error { if !gopts.NoLock { var lock *restic.Lock - lock, ctx, err = lockRepo(ctx, repo) + lock, ctx, err = lockRepo(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/cmd_tag.go b/cmd/restic/cmd_tag.go index e5948ea02..fe4638547 100644 --- a/cmd/restic/cmd_tag.go +++ b/cmd/restic/cmd_tag.go @@ -111,7 +111,7 @@ func runTag(ctx context.Context, opts TagOptions, gopts GlobalOptions, args []st if !gopts.NoLock { Verbosef("create exclusive lock for repository\n") var lock *restic.Lock - lock, ctx, err = lockRepoExclusive(ctx, repo) + lock, ctx, err = lockRepoExclusive(ctx, repo, gopts.RetryLock, gopts.JSON) defer unlockRepo(lock) if err != nil { return err diff --git a/cmd/restic/global.go b/cmd/restic/global.go index c2c288421..32f18a67f 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -59,6 +59,7 @@ type GlobalOptions struct { Quiet bool Verbose int NoLock bool + RetryLock time.Duration JSON bool CacheDir string NoCache bool @@ -115,6 +116,7 @@ func init() { // use empty paremeter name as `-v, --verbose n` instead of the correct `--verbose=n` is confusing f.CountVarP(&globalOptions.Verbose, "verbose", "v", "be verbose (specify multiple times or a level using --verbose=n``, max level/times is 2)") f.BoolVar(&globalOptions.NoLock, "no-lock", false, "do not lock the repository, this allows some operations on read-only repositories") + f.DurationVar(&globalOptions.RetryLock, "retry-lock", 0, "retry to lock the repository if it is already locked, takes a value like 5m or 2h (default: no retries)") f.BoolVarP(&globalOptions.JSON, "json", "", false, "set output mode to JSON for commands that support it") f.StringVar(&globalOptions.CacheDir, "cache-dir", "", "set the cache `directory`. (default: use system default cache directory)") f.BoolVar(&globalOptions.NoCache, "no-cache", false, "do not use a local cache") diff --git a/cmd/restic/lock.go b/cmd/restic/lock.go index f39a08db6..450922704 100644 --- a/cmd/restic/lock.go +++ b/cmd/restic/lock.go @@ -21,17 +21,29 @@ var globalLocks struct { sync.Once } -func lockRepo(ctx context.Context, repo restic.Repository) (*restic.Lock, context.Context, error) { - return lockRepository(ctx, repo, false) +func lockRepo(ctx context.Context, repo restic.Repository, retryLock time.Duration, json bool) (*restic.Lock, context.Context, error) { + return lockRepository(ctx, repo, false, retryLock, json) } -func lockRepoExclusive(ctx context.Context, repo restic.Repository) (*restic.Lock, context.Context, error) { - return lockRepository(ctx, repo, true) +func lockRepoExclusive(ctx context.Context, repo restic.Repository, retryLock time.Duration, json bool) (*restic.Lock, context.Context, error) { + return lockRepository(ctx, repo, true, retryLock, json) +} + +var ( + retrySleepStart = 5 * time.Second + retrySleepMax = 60 * time.Second +) + +func minDuration(a, b time.Duration) time.Duration { + if a <= b { + return a + } + return b } // lockRepository wraps the ctx such that it is cancelled when the repository is unlocked // cancelling the original context also stops the lock refresh -func lockRepository(ctx context.Context, repo restic.Repository, exclusive bool) (*restic.Lock, context.Context, error) { +func lockRepository(ctx context.Context, repo restic.Repository, exclusive bool, retryLock time.Duration, json bool) (*restic.Lock, context.Context, error) { // make sure that a repository is unlocked properly and after cancel() was // called by the cleanup handler in global.go globalLocks.Do(func() { @@ -43,7 +55,44 @@ func lockRepository(ctx context.Context, repo restic.Repository, exclusive bool) lockFn = restic.NewExclusiveLock } - lock, err := lockFn(ctx, repo) + var lock *restic.Lock + var err error + + retrySleep := minDuration(retrySleepStart, retryLock) + retryMessagePrinted := false + retryTimeout := time.After(retryLock) + +retryLoop: + for { + lock, err = lockFn(ctx, repo) + if err != nil && restic.IsAlreadyLocked(err) { + + if !retryMessagePrinted { + if !json { + Verbosef("repo already locked, waiting up to %s for the lock\n", retryLock) + } + retryMessagePrinted = true + } + + debug.Log("repo already locked, retrying in %v", retrySleep) + retrySleepCh := time.After(retrySleep) + + select { + case <-ctx.Done(): + return nil, ctx, ctx.Err() + case <-retryTimeout: + debug.Log("repo already locked, timeout expired") + // Last lock attempt + lock, err = lockFn(ctx, repo) + break retryLoop + case <-retrySleepCh: + retrySleep = minDuration(retrySleep*2, retrySleepMax) + } + } else { + // anything else, either a successful lock or another error + break retryLoop + } + } if restic.IsInvalidLock(err) { return nil, ctx, errors.Fatalf("%v\n\nthe `unlock --remove-all` command can be used to remove invalid locks. Make sure that no other restic process is accessing the repository when running the command", err) } diff --git a/cmd/restic/lock_test.go b/cmd/restic/lock_test.go index c074f15a6..ad9161739 100644 --- a/cmd/restic/lock_test.go +++ b/cmd/restic/lock_test.go @@ -3,11 +3,13 @@ package main import ( "context" "fmt" + "strings" "testing" "time" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/test" rtest "github.com/restic/restic/internal/test" ) @@ -23,8 +25,8 @@ func openTestRepo(t *testing.T, wrapper backendWrapper) (*repository.Repository, return repo, cleanup, env } -func checkedLockRepo(ctx context.Context, t *testing.T, repo restic.Repository) (*restic.Lock, context.Context) { - lock, wrappedCtx, err := lockRepo(ctx, repo) +func checkedLockRepo(ctx context.Context, t *testing.T, repo restic.Repository, env *testEnvironment) (*restic.Lock, context.Context) { + lock, wrappedCtx, err := lockRepo(ctx, repo, env.gopts.RetryLock, env.gopts.JSON) rtest.OK(t, err) rtest.OK(t, wrappedCtx.Err()) if lock.Stale() { @@ -34,10 +36,10 @@ func checkedLockRepo(ctx context.Context, t *testing.T, repo restic.Repository) } func TestLock(t *testing.T) { - repo, cleanup, _ := openTestRepo(t, nil) + repo, cleanup, env := openTestRepo(t, nil) defer cleanup() - lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo) + lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo, env) unlockRepo(lock) if wrappedCtx.Err() == nil { t.Fatal("unlock did not cancel context") @@ -45,12 +47,12 @@ func TestLock(t *testing.T) { } func TestLockCancel(t *testing.T) { - repo, cleanup, _ := openTestRepo(t, nil) + repo, cleanup, env := openTestRepo(t, nil) defer cleanup() ctx, cancel := context.WithCancel(context.Background()) defer cancel() - lock, wrappedCtx := checkedLockRepo(ctx, t, repo) + lock, wrappedCtx := checkedLockRepo(ctx, t, repo, env) cancel() if wrappedCtx.Err() == nil { t.Fatal("canceled parent context did not cancel context") @@ -61,10 +63,10 @@ func TestLockCancel(t *testing.T) { } func TestLockUnlockAll(t *testing.T) { - repo, cleanup, _ := openTestRepo(t, nil) + repo, cleanup, env := openTestRepo(t, nil) defer cleanup() - lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo) + lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo, env) _, err := unlockAll(0) rtest.OK(t, err) if wrappedCtx.Err() == nil { @@ -81,10 +83,10 @@ func TestLockConflict(t *testing.T) { repo2, err := OpenRepository(context.TODO(), env.gopts) rtest.OK(t, err) - lock, _, err := lockRepoExclusive(context.Background(), repo) + lock, _, err := lockRepoExclusive(context.Background(), repo, env.gopts.RetryLock, env.gopts.JSON) rtest.OK(t, err) defer unlockRepo(lock) - _, _, err = lockRepo(context.Background(), repo2) + _, _, err = lockRepo(context.Background(), repo2, env.gopts.RetryLock, env.gopts.JSON) if err == nil { t.Fatal("second lock should have failed") } @@ -104,7 +106,7 @@ func (b *writeOnceBackend) Save(ctx context.Context, h restic.Handle, rd restic. } func TestLockFailedRefresh(t *testing.T) { - repo, cleanup, _ := openTestRepo(t, func(r restic.Backend) (restic.Backend, error) { + repo, cleanup, env := openTestRepo(t, func(r restic.Backend) (restic.Backend, error) { return &writeOnceBackend{Backend: r}, nil }) defer cleanup() @@ -117,7 +119,7 @@ func TestLockFailedRefresh(t *testing.T) { refreshInterval, refreshabilityTimeout = ri, rt }() - lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo) + lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo, env) select { case <-wrappedCtx.Done(): @@ -140,7 +142,7 @@ func (b *loggingBackend) Save(ctx context.Context, h restic.Handle, rd restic.Re } func TestLockSuccessfulRefresh(t *testing.T) { - repo, cleanup, _ := openTestRepo(t, func(r restic.Backend) (restic.Backend, error) { + repo, cleanup, env := openTestRepo(t, func(r restic.Backend) (restic.Backend, error) { return &loggingBackend{ Backend: r, t: t, @@ -157,7 +159,7 @@ func TestLockSuccessfulRefresh(t *testing.T) { refreshInterval, refreshabilityTimeout = ri, rt }() - lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo) + lock, wrappedCtx := checkedLockRepo(context.Background(), t, repo, env) select { case <-wrappedCtx.Done(): @@ -168,3 +170,74 @@ func TestLockSuccessfulRefresh(t *testing.T) { // unlockRepo should not crash unlockRepo(lock) } + +func TestLockWaitTimeout(t *testing.T) { + repo, cleanup, env := openTestRepo(t, nil) + defer cleanup() + + elock, _, err := lockRepoExclusive(context.TODO(), repo, env.gopts.RetryLock, env.gopts.JSON) + test.OK(t, err) + + retryLock := 100 * time.Millisecond + + start := time.Now() + lock, _, err := lockRepo(context.TODO(), repo, retryLock, env.gopts.JSON) + duration := time.Since(start) + + test.Assert(t, err != nil, + "create normal lock with exclusively locked repo didn't return an error") + test.Assert(t, strings.Contains(err.Error(), "repository is already locked exclusively"), + "create normal lock with exclusively locked repo didn't return the correct error") + test.Assert(t, retryLock <= duration && duration < retryLock+5*time.Millisecond, + "create normal lock with exclusively locked repo didn't wait for the specified timeout") + + test.OK(t, lock.Unlock()) + test.OK(t, elock.Unlock()) +} +func TestLockWaitCancel(t *testing.T) { + repo, cleanup, env := openTestRepo(t, nil) + defer cleanup() + + elock, _, err := lockRepoExclusive(context.TODO(), repo, env.gopts.RetryLock, env.gopts.JSON) + test.OK(t, err) + + retryLock := 100 * time.Millisecond + cancelAfter := 40 * time.Millisecond + + ctx, cancel := context.WithCancel(context.TODO()) + time.AfterFunc(cancelAfter, cancel) + + start := time.Now() + lock, _, err := lockRepo(ctx, repo, retryLock, env.gopts.JSON) + duration := time.Since(start) + + test.Assert(t, err != nil, + "create normal lock with exclusively locked repo didn't return an error") + test.Assert(t, strings.Contains(err.Error(), "context canceled"), + "create normal lock with exclusively locked repo didn't return the correct error") + test.Assert(t, cancelAfter <= duration && duration < cancelAfter+5*time.Millisecond, + "create normal lock with exclusively locked repo didn't return in time") + + test.OK(t, lock.Unlock()) + test.OK(t, elock.Unlock()) +} + +func TestLockWaitSuccess(t *testing.T) { + repo, cleanup, env := openTestRepo(t, nil) + defer cleanup() + + elock, _, err := lockRepoExclusive(context.TODO(), repo, env.gopts.RetryLock, env.gopts.JSON) + test.OK(t, err) + + retryLock := 100 * time.Millisecond + unlockAfter := 40 * time.Millisecond + + time.AfterFunc(unlockAfter, func() { + test.OK(t, elock.Unlock()) + }) + + lock, _, err := lockRepo(context.TODO(), repo, retryLock, env.gopts.JSON) + test.OK(t, err) + + test.OK(t, lock.Unlock()) +} diff --git a/doc/design.rst b/doc/design.rst index 7102585ac..94dabdc34 100644 --- a/doc/design.rst +++ b/doc/design.rst @@ -603,7 +603,10 @@ that the process is dead and considers the lock to be stale. When a new lock is to be created and no other conflicting locks are detected, restic creates a new lock, waits, and checks if other locks appeared in the repository. Depending on the type of the other locks and -the lock to be created, restic either continues or fails. +the lock to be created, restic either continues or fails. If the +``--retry-lock`` option is specified, restic will retry +creating the lock periodically until it succeeds or the specified +timeout expires. Read and Write Ordering ======================= diff --git a/doc/manual_rest.rst b/doc/manual_rest.rst index 3bf2c475f..7f812f4e0 100644 --- a/doc/manual_rest.rst +++ b/doc/manual_rest.rst @@ -66,6 +66,7 @@ Usage help is available: -q, --quiet do not output comprehensive progress report -r, --repo repository repository to backup to or restore from (default: $RESTIC_REPOSITORY) --repository-file file file to read the repository location from (default: $RESTIC_REPOSITORY_FILE) + --retry-lock duration retry to lock the repository if it is already locked, takes a value like 5m or 2h (default: no retries) --tls-client-cert file path to a file containing PEM encoded TLS client certificate and private key -v, --verbose be verbose (specify multiple times or a level using --verbose=n, max level/times is 2) @@ -141,6 +142,7 @@ command: -q, --quiet do not output comprehensive progress report -r, --repo repository repository to backup to or restore from (default: $RESTIC_REPOSITORY) --repository-file file file to read the repository location from (default: $RESTIC_REPOSITORY_FILE) + --retry-lock duration retry to lock the repository if it is already locked, takes a value like 5m or 2h (default: no retries) --tls-client-cert file path to a file containing PEM encoded TLS client certificate and private key -v, --verbose be verbose (specify multiple times or a level using --verbose=n, max level/times is 2)