From 23ebec717c61f42500ad91f3ff0d2a017b8dbe38 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 30 Jul 2022 12:57:18 +0200 Subject: [PATCH 1/3] Remove stale comments from backend/sftp The preExec and postExec functions were removed in 0bdb131521f84ebce3541f8ccd684c93892b5e66 from 2018. --- internal/backend/sftp/sftp.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index 71cdcaa24..e0edb807c 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -116,8 +116,7 @@ func (r *SFTP) clientError() error { } // Open opens an sftp backend as described by the config by running -// "ssh" with the appropriate arguments (or cfg.Command, if set). The function -// preExec is run just before, postExec just after starting a program. +// "ssh" with the appropriate arguments (or cfg.Command, if set). func Open(ctx context.Context, cfg Config) (*SFTP, error) { debug.Log("open backend with config %#v", cfg) @@ -214,8 +213,7 @@ func buildSSHCommand(cfg Config) (cmd string, args []string, err error) { } // Create creates an sftp backend as described by the config by running "ssh" -// with the appropriate arguments (or cfg.Command, if set). The function -// preExec is run just before, postExec just after starting a program. +// with the appropriate arguments (or cfg.Command, if set). func Create(ctx context.Context, cfg Config) (*SFTP, error) { cmd, args, err := buildSSHCommand(cfg) if err != nil { From 2bdc40e6121d26b9d0897660b513bab3a76184fa Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 30 Jul 2022 12:24:04 +0200 Subject: [PATCH 2/3] Speed up restic init over slow SFTP links pkg/sftp.Client.MkdirAll(d) does a Stat to determine if d exists and is a directory, then a recursive call to create the parent, so the calls for data/?? each take three round trips. Doing a Mkdir first should eliminate two round trips for 255/256 data directories as well as all but one of the top-level directories. Also, we can do all of the calls concurrently. This may reintroduce some of the Stat calls when multiple goroutines try to create the same parent, but at the default number of connections, that should not be much of a problem. --- go.mod | 2 +- go.sum | 3 ++- internal/backend/sftp/sftp.go | 29 ++++++++++++++++++++++------- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 81d35692f..a2c301d15 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( golang.org/x/crypto v0.0.0-20220321153916-2c7772ba3064 golang.org/x/net v0.0.0-20220325170049-de3da57026de golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a - golang.org/x/sync v0.0.0-20210220032951-036812b2e83c + golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 golang.org/x/sys v0.0.0-20220325203850-36772127a21f golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 golang.org/x/text v0.3.7 diff --git a/go.sum b/go.sum index 4f1ff9309..54711322a 100644 --- a/go.sum +++ b/go.sum @@ -439,8 +439,9 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index e0edb807c..0aa2700a6 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -21,6 +21,7 @@ import ( "github.com/cenkalti/backoff/v4" "github.com/pkg/sftp" + "golang.org/x/sync/errgroup" ) // SFTP is a backend in a directory accessed via SFTP. @@ -154,15 +155,29 @@ func Open(ctx context.Context, cfg Config) (*SFTP, error) { return sftp, nil } -func (r *SFTP) mkdirAllDataSubdirs() error { +func (r *SFTP) mkdirAllDataSubdirs(ctx context.Context, nconn uint) error { + // Run multiple MkdirAll calls concurrently. These involve multiple + // round-trips and we do a lot of them, so this whole operation can be slow + // on high-latency links. + g, _ := errgroup.WithContext(ctx) + // Use errgroup's built-in semaphore, because r.sem is not initialized yet. + g.SetLimit(int(nconn)) + for _, d := range r.Paths() { - err := r.c.MkdirAll(d) - if err != nil { - return err - } + d := d + g.Go(func() error { + // First try Mkdir. For most directories in Paths, this takes one + // round trip, not counting duplicate parent creations causes by + // concurrency. MkdirAll first does Stat, then recursive MkdirAll + // on the parent, so calls typically take three round trips. + if err := r.c.Mkdir(d); err == nil { + return nil + } + return r.c.MkdirAll(d) + }) } - return nil + return g.Wait() } // Join combines path components with slashes (according to the sftp spec). @@ -240,7 +255,7 @@ func Create(ctx context.Context, cfg Config) (*SFTP, error) { } // create paths for data and refs - if err = sftp.mkdirAllDataSubdirs(); err != nil { + if err = sftp.mkdirAllDataSubdirs(ctx, cfg.Connections); err != nil { return nil, err } From b9fa6e05bd3769ba369e73397cebc0ef9f2696b9 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 30 Jul 2022 22:53:52 +0200 Subject: [PATCH 3/3] Add changelog for #3837/#3840 --- changelog/unreleased/issue-3837 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/issue-3837 diff --git a/changelog/unreleased/issue-3837 b/changelog/unreleased/issue-3837 new file mode 100644 index 000000000..11e06d953 --- /dev/null +++ b/changelog/unreleased/issue-3837 @@ -0,0 +1,8 @@ +Enhancement: SFTP backend initialization is faster on slow links + +Restic init on an SFTP backend now sends multiple mkdir commands to the +backend concurrently, to reduce the wait when creating a repository +over a very slow link. + +https://github.com/restic/restic/issues/3837 +https://github.com/restic/restic/pull/3840