Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allocation bug in S3 downloader still affect Lambda resources, Golang GC and increase costs for endusers #1211

Closed
3 tasks done
vvelikodny opened this issue Apr 6, 2021 · 1 comment
Labels
bug This issue is a bug. closed-for-staleness response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@vvelikodny
Copy link
Contributor

vvelikodny commented Apr 6, 2021

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug

Hello, AWS team! This kind of bug could be avoided but still presented in the SDK and affects resource consumption and cost for the end-users (AWS Lambda for example or even another provider) so be worth considering.

The root cause of this bug described here #482 but has been ignored for a long time and closed automatically. It's in continuation of PR #1171 which has been added recently to documentation to describe possible issues.

Just noticed still getting points on that issue on StackOverflow recently and the downloader works well only with a pre-allocated buffer, although changes could fix it and weren't implemented in new SDK versions. Lambda function with v.1.2.1 still overconsume memory if the client doesn't pass proper buffer.

Results of downloading 150MB file, pay attention to Billed Duration and Max Memory Used:

  • SDK v1.2.1
    Downloaded 113116549 bytes REPORT RequestId: edd252d4-499b-4536-906c-031a67861104 Duration: 26066.62 ms Billed Duration: 26067 ms Memory Size: 3008 MB Max Memory Used: 712 MB Init Duration: 89.58 ms

  • SDK v1.2.1 pre-allocated buf:
    Downloaded 113116549 bytes REPORT RequestId: a6a73113-f50c-4077-b297-b3799bb1ceee Duration: 474.38 ms Billed Duration: 475 ms Memory Size: 3008 MB Max Memory Used: 162 MB Init Duration: 86.49 ms

vs

  • old SDK with commit changes and almost the same as with pre-allocated
    Downloaded 113116549 bytes REPORT RequestId: 7f219cd9-f0c7-43bf-abc4-111875643ff4 Duration: 509.86 ms Billed Duration: 510 ms Memory Size: 3008 MB Max Memory Used: 285 MB Init Duration: 95.03 ms

PR with fix #483

Version of AWS SDK for Go?
Example: lastest from the main branch

Version of Go (go version)?
v1.1.15
To Reproduce (observed behavior)
Steps to reproduce the behavior (please share code or minimal repo)

https://gist.github.com/vvelikodny/db3d2e54ab5fdb97a2cce6b6842f67cc

package main

import (
	"context"
	"fmt"
	"time"

	lambdaevents "github.com/aws/aws-lambda-go/events"
	"github.com/aws/aws-lambda-go/lambda"
	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	s3manager "github.com/aws/aws-sdk-go-v2/feature/s3/manager"
	"github.com/aws/aws-sdk-go-v2/service/s3"
)

func handleDemoEvent(ctx context.Context, s3Event lambdaevents.S3Event) error {
	fmt.Println("New SDK write to file")

	for _, record := range s3Event.Records {
		return HandleDemoRecord(ctx, record.S3.Bucket.Name, record.AWSRegion, record.S3.Object.Key)
	}

	return nil
}

func HandleDemoRecord(ctx context.Context, bucket, region, item string) error {
	fmt.Printf("Bucket = %s, Key = %s\n", bucket, item)

	cfg, err := config.LoadDefaultConfig(ctx)
	if err != nil {
		return err
	}
	cfg.Region = region

	headObject, err := getHeadObject(ctx, bucket, item, cfg)
	if err != nil {
		return err
	}

	fmt.Printf("obj: %v\nbytes:%+v\n", headObject, headObject.ContentLength)

	downloader := s3manager.NewDownloader(s3.NewFromConfig(cfg))

	//var buf []byte
	// allocate before
	buf := make([]byte, int(headObject.ContentLength))

	start := time.Now()
	fmt.Println("Starting download of file, ", start)
	w := s3manager.NewWriteAtBuffer(buf)
	numBytesDownloaded, err := downloader.Download(ctx, w, &s3.GetObjectInput{
		Bucket: aws.String(bucket),
		Key:    aws.String(item),
	})

	if err != nil {
		return err
	}

	fmt.Println("Downloaded ", numBytesDownloaded, " bytes, took ", time.Since(start))
	return nil
}

func getHeadObject(ctx context.Context, bucket string, key string, config aws.Config) (*s3.HeadObjectOutput, error) {
	client := s3.NewFromConfig(config)

	input := &s3.HeadObjectInput{
		Bucket: aws.String(bucket),
		Key:    aws.String(key),
	}

	headObjectResponse, err := client.HeadObject(ctx, input)
	if err != nil {
		return nil, err
	}

	fmt.Printf("Downloaded: %v\n", headObjectResponse.ContentLength)

	return headObjectResponse, nil
}

func main() {
	lambda.Start(handleDemoEvent)
}

Expected behavior
Allocate a proper amount of memory (less memory) and time to download big files.

@vvelikodny vvelikodny added the bug This issue is a bug. label Apr 6, 2021
@vvelikodny vvelikodny changed the title allocation bug in S3 downloader affect Lambda resources & increase costs for endusers Allocation bug in S3 downloader still affect Lambda resources, Golang GC and increase costs for endusers Apr 6, 2021
@vudh1
Copy link
Contributor

vudh1 commented Jun 1, 2022

Hi, can you confirm if this is still persisting with the latest version of SDK?

@vudh1 vudh1 added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 1, 2022
@github-actions github-actions bot added closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 7, 2022
@github-actions github-actions bot closed this as completed Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closed-for-staleness response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants