s3manager.Uploader can spin up more than Concurrency goroutines

[jlburkhead]

Version of AWS SDK for Go?

v1.10.20

Version of Go (go version)?

go1.8.3

What issue did you see?

The docs for s3manager.Uploader are unclear that the Concurrency field is a limit on the number of goroutines to spin up per invocation of the Uploader.Upload* methods as opposed to a total number of goroutines the uploader will spin up. Since the option is set on the Uploader structure which is safe to use across multiple concurrent goroutines,

It is safe to call Upload() on this structure for multiple objects and across concurrent goroutines.

I would expect the Concurrency field to be a total limit on number of goroutines the uploader creates even if Upload* is called more than once concurrently. Is this just unclear documentation or a bug in the implementation? Even if the current behavior is expected it would be helpful to update the docs to be clear, although I think the Concurrency field is much less useful if it is a per invocation limit.

Steps to reproduce

Easy to see that this is how the uploader is implemented

Little program to show the number of goroutines started by concurrent calls to Upload

package main

import (
	"flag"
	"log"
	"runtime"
	"strconv"
	"time"

	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/s3"
	"github.com/aws/aws-sdk-go/service/s3/s3manager"
)

type endlessReader struct{}

func (r *endlessReader) Read(p []byte) (int, error) {
	return len(p), nil
}

func main() {

	bucket := flag.String("bucket", "", "s3 to upload to bucket")
	prefix := flag.String("prefix", "", "key prefix to upload to")
	concurrency := flag.Int("concurrency", 5, "number of concurrent calls to Upload")
	profile := flag.String("profile", "", "aws credential profile to use")
	flag.Parse()

	go func() {
		ticker := time.NewTicker(5 * time.Second)
		for {
			<-ticker.C
			log.Print(runtime.NumGoroutine())
		}
	}()

	svc := s3.New(session.Must(session.NewSessionWithOptions(session.Options{
		Config:  aws.Config{Region: aws.String("us-east-1")},
		Profile: *profile,
	})))
	uploader := s3manager.NewUploaderWithClient(svc, func(uploader *s3manager.Uploader) {
		uploader.Concurrency = 10 // 10 concurrent requests
	})

	for i := 0; i < *concurrency; i++ {
		go func(i int) {
			_, err := uploader.Upload(&s3manager.UploadInput{
				Bucket: bucket,
				Key:    aws.String(*prefix + strconv.Itoa(i)),
				Body:   new(endlessReader),
			})
			if err != nil {
				log.Fatal(err)
			}
		}(i)
	}

	done := make(chan struct{})
	<-done
}

[xibz]

Hello @jlburkhead, thank you for reaching out to us. The documentation is a little unclear and it should be stated that the max concurrent goroutines is per invocation. I will go ahead and add this in the backlog to better document that.