From 0268d0e7d6ae43e2cb46abdecdee8d55439f744e Mon Sep 17 00:00:00 2001 From: George Armhold Date: Thu, 2 Nov 2017 18:29:32 -0400 Subject: [PATCH] swift backend: limit http concurrency in Save(), Stat(), Test(), Remove(), List(). move comment regarding problematic List() backend api (it's s3's ListObjects that has a problem, NOT swift's ObjectsWalk). As per discussion in PR #1399. --- internal/backend/s3/s3.go | 3 +++ internal/backend/swift/swift.go | 25 +++++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go index 86fd078a5..844e832d2 100644 --- a/internal/backend/s3/s3.go +++ b/internal/backend/s3/s3.go @@ -433,6 +433,9 @@ func (be *Backend) List(ctx context.Context, t restic.FileType) <-chan string { prefix += "/" } + // NB: unfortunately we can't protect this with be.sem.GetToken() here. + // Doing so would enable a deadlock situation (gh-1399), as ListObjects() + // starts its own goroutine and returns results via a channel. listresp := be.client.ListObjects(be.cfg.Bucket, prefix, true, ctx.Done()) go func() { diff --git a/internal/backend/swift/swift.go b/internal/backend/swift/swift.go index 3497fec1d..5ada1f5d8 100644 --- a/internal/backend/swift/swift.go +++ b/internal/backend/swift/swift.go @@ -163,6 +163,9 @@ func (be *beSwift) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err debug.Log("Save %v at %v", h, objName) + be.sem.GetToken() + defer be.sem.ReleaseToken() + // Check key does not already exist switch _, _, err = be.conn.Object(be.container, objName); err { case nil: @@ -176,11 +179,6 @@ func (be *beSwift) Save(ctx context.Context, h restic.Handle, rd io.Reader) (err return errors.Wrap(err, "conn.Object") } - be.sem.GetToken() - defer func() { - be.sem.ReleaseToken() - }() - encoding := "binary/octet-stream" debug.Log("PutObject(%v, %v, %v)", be.container, objName, encoding) @@ -196,6 +194,9 @@ func (be *beSwift) Stat(ctx context.Context, h restic.Handle) (bi restic.FileInf objName := be.Filename(h) + be.sem.GetToken() + defer be.sem.ReleaseToken() + obj, _, err := be.conn.Object(be.container, objName) if err != nil { debug.Log("Object() err %v", err) @@ -208,6 +209,10 @@ func (be *beSwift) Stat(ctx context.Context, h restic.Handle) (bi restic.FileInf // Test returns true if a blob of the given type and name exists in the backend. func (be *beSwift) Test(ctx context.Context, h restic.Handle) (bool, error) { objName := be.Filename(h) + + be.sem.GetToken() + defer be.sem.ReleaseToken() + switch _, _, err := be.conn.Object(be.container, objName); err { case nil: return true, nil @@ -223,6 +228,10 @@ func (be *beSwift) Test(ctx context.Context, h restic.Handle) (bool, error) { // Remove removes the blob with the given name and type. func (be *beSwift) Remove(ctx context.Context, h restic.Handle) error { objName := be.Filename(h) + + be.sem.GetToken() + defer be.sem.ReleaseToken() + err := be.conn.ObjectDelete(be.container, objName) debug.Log("Remove(%v) -> err %v", h, err) return errors.Wrap(err, "conn.ObjectDelete") @@ -240,12 +249,12 @@ func (be *beSwift) List(ctx context.Context, t restic.FileType) <-chan string { go func() { defer close(ch) - // NB: unfortunately we can't protect this with be.sem.GetToken() here. - // Doing so would enable a deadlock situation (PR: gh-1399), as ObjectsWalk() - // starts its own goroutine and returns results via a channel. err := be.conn.ObjectsWalk(be.container, &swift.ObjectsOpts{Prefix: prefix}, func(opts *swift.ObjectsOpts) (interface{}, error) { + be.sem.GetToken() newObjects, err := be.conn.ObjectNames(be.container, opts) + be.sem.ReleaseToken() + if err != nil { return nil, errors.Wrap(err, "conn.ObjectNames") }