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

Severe Performance Degradation for S3 using SDK 1.8.32 #1440

Closed
2 tasks done
lintonv opened this issue Aug 20, 2020 · 10 comments
Closed
2 tasks done

Severe Performance Degradation for S3 using SDK 1.8.32 #1440

lintonv opened this issue Aug 20, 2020 · 10 comments
Assignees
Labels
closing-soon This issue will automatically close in 4 days unless further comments are made. guidance Question that needs advice or information.

Comments

@lintonv
Copy link

lintonv commented Aug 20, 2020

Confirm by changing [ ] to [x] below:

Platform/OS/Hardware/Device
What are you running the sdk on?
Ubuntu 18.04 (x86_64)

Describe the question
We have been using SDK version 1.7.336 for a while. But we need some of the new HttpClient features in 1.8. So we upgraded to 1.8.32.

We saw our S3 transactions - puts, gets, lists all decrease in performance significantly

For example - for uploading data to S3 -

SDK 1.7.336, Speed - 1.5G to 1.75G
SDK 1.8.32, Speed - 50M to 150M 

This is a significant performance regression

Here's the build info (we use bazel)

aws_copts = [
    "-DPLATFORM_LINUX",
    "-DAWS_SDK_VERSION_MAJOR=1",
    "-DAWS_SDK_VERSION_MINOR=8",
    "-DAWS_SDK_VERSION_PATCH=32",
    "-DENABLE_CURL_CLIENT",
    "-DENABLE_OPENSSL_ENCRYPTION",
]

How do I track where the problem is?

Logs/output
Nothing from a logs point of view

@lintonv lintonv added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Aug 20, 2020
@KaibaLopez
Copy link
Contributor

Hi @lintonv ,
That's interesting, would it be possible for you to share the code you're using to test these?

@KaibaLopez KaibaLopez added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Aug 20, 2020
@KaibaLopez KaibaLopez self-assigned this Aug 20, 2020
@lintonv
Copy link
Author

lintonv commented Aug 21, 2020

Hi @KaibaLopez

This is the gist of the code -

void S3Storage::upload_file(const std::string &bucket_name,
                            const std::string &object_key,
                            const std::string &file_path) const
{

    Aws::S3::Model::PutObjectRequest object_request;

    object_request.WithBucket(to_aws_str(bucket_name)).WithKey(to_aws_str(object_key));

    // The allocation tag is used for memory tracking purposes only - this char* could be anything
    // See Aws::MakeShared usage for details.
    constexpr auto mem_alloc_tag = "fstream_sptr";
    
    auto input_data = Aws::MakeShared<Aws::Utils::FStreamWithFileName>(
        mem_alloc_tag, to_aws_str(file_path), std::ios_base::in | std::ios_base::binary);
    
    object_request.SetBody(input_data);
    
    auto tag = set_upload_request_properties(object_request, object_key);

    auto path    = fmt::format("s3://{}/{}", bucket_name, object_key);
    // retry logic
    auto outcome = with_retry([this, &object_request]() { return this->s3_client_->PutObject(object_request); }, path);

    if (!outcome.IsSuccess())
    {
        // debug stmt
    }

    try
    {
        add_tag_to_object(bucket_name, object_key, tag.first, tag.second);
    }
    catch (S3Exception &e)
    {
        //debug stmt
    }
}

S3 Client setup

    Aws::Client::ClientConfiguration client_cfg;
    client_cfg.lowSpeedLimit = 3000000;

    s3_client_ = std::make_shared<Aws::S3::S3Client>(client_cfg);

This is the same code we use for SDK Version 1.7.336 (and the latest version 1.8.32)

@lintonv
Copy link
Author

lintonv commented Aug 21, 2020

@KaibaLopez - I have root caused this.

When I add setenv("AWS_EC2_METADATA_DISABLED", "true", 1); after Aws::InitAPI() the performance regression no longer exists.

Can you explain why? And is there a better solution other than what I have determined?

@KaibaLopez
Copy link
Contributor

KaibaLopez commented Aug 21, 2020

ahh, ok so I'm almost certain I know what the issue is then:

What's going on there is that since version 1.8 we've changed the Aws::Client::ClientConfiguration default constructor to search for the region (where as before 1.8 we had a hard coded default of "us-east-1"). Part of that change is looking through the ec2 metadata, which can take a long time. By having AWS_EC2_METADATA_DISABLED it avoids doing this search.

And is there a better solution other than what I have determined?

Not currently, that is actually the workaround we recommend.

@lintonv
Copy link
Author

lintonv commented Aug 24, 2020

Thank you @KaibaLopez. Was this documented somewhere? Did I just miss that? If it is not documented, could you please make sure it is?

@lintonv lintonv closed this as completed Aug 24, 2020
@lintonv lintonv reopened this Aug 27, 2020
@lintonv
Copy link
Author

lintonv commented Aug 27, 2020

@KaibaLopez - we saw segfaults and other issues with that flag on EC2 environments. We have a system that has a mixture of code running on EC2 and running outside AWS but depending on S3 clients.

Is there an alternative way to not use that flag? Maybe configure the client in a different way?

@KaibaLopez
Copy link
Contributor

KaibaLopez commented Aug 27, 2020

@lintonv ,
Sorry we are working on a better way to do it but currently the only way to avoid these involve environment variables. If the AWS_EC2_METADATA_DISABLED is problematic for you, you could also set AWS_DEFAULT_REGION or AWS_REGION to your region...
Though honestly if having the env variable as true causes troubles later on, the best workaround I can think of is having something like:

setenv("AWS_EC2_METADATA_DISABLED", "true", 1);
Aws::Client::ClientConfiguration clientConfig("default"); // use this configuration for all aws clients or do the same for other client configurations
setenv("AWS_EC2_METADATA_DISABLED", "false", 1);

either that or changing the source code directly- on clientConfiguration.cpp remove lines 127-134.
Hope one of these works for you.
Ah and also, we have this documented as part of the changes on version 1.8 in our wiki

@lintonv
Copy link
Author

lintonv commented Aug 27, 2020

Thanks @KaibaLopez
What will be the negative impact of commenting out the lines in clientConfiguration.cpp?
I understand that removing those lines will remove the need for the ec2 metadata check, but is there any negative impact of that for the code in version 1.8?

@KaibaLopez KaibaLopez added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Aug 27, 2020
@KaibaLopez
Copy link
Contributor

Not really, it just means that it won't do it for you.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Aug 28, 2020
@KaibaLopez KaibaLopez added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Aug 28, 2020
@lintonv lintonv closed this as completed Aug 31, 2020
@TimLand
Copy link

TimLand commented Apr 22, 2021

mark

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closing-soon This issue will automatically close in 4 days unless further comments are made. guidance Question that needs advice or information.
Projects
None yet
Development

No branches or pull requests

3 participants