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

Aws retry settings are not working #3131

Closed
Manjunathagopi opened this issue Sep 30, 2024 · 5 comments
Closed

Aws retry settings are not working #3131

Manjunathagopi opened this issue Sep 30, 2024 · 5 comments
Assignees
Labels
bug This issue is a bug. investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 This is a standard priority issue

Comments

@Manjunathagopi
Copy link

Manjunathagopi commented Sep 30, 2024

Can someone explain how this retry strategy works, i configured timeout configs to 100ms and retry strategy to NO_RETRY. But still i am seeing requests are not getting cancelled and time to get object is more than 1 second.

sample code snippet.

Aws::S3Crt::ClientConfiguration config;
 config.httpRequestTimeoutMs = 100;
 config.connectTimeoutMs = 100;
 config.requestTimeoutMs = 100;  
 config.crtRetryStrategyConfig.crtRetryStrategyType = 
 Aws::S3Crt::S3CrtClientConfiguration::CrtRetryStrategyConfig::CrtRetryStrategyType::NO_RETRY;
 config.throughputTargetGbps = 1;
 config.partSize = 10*1024*1024;
 s3_crt_client = Aws::New<Aws::S3Crt::S3CrtClient>("test", config);

aws cpp SDK version : 1.11.408

Originally posted by @Manjunathagopi in #2594 (comment)

@Manjunathagopi Manjunathagopi changed the title Hi, Aws retry settings are not working Sep 30, 2024
@jmklix
Copy link
Member

jmklix commented Oct 4, 2024

  • httpReqTimeout is not used in s3crt
  • connectTimeoutMs is used as a limit to how long to wait before the tls connection is established
  • requestTimeoutMs is used but it's complicated. You can see it's usage here

@jmklix jmklix added guidance Question that needs advice or information. duplicate This issue is a duplicate. closing-soon This issue will automatically close in 4 days unless further comments are made. p3 This is a minor priority issue labels Oct 4, 2024
@Manjunathagopi
Copy link
Author

Hi @jmklix thanks for the response. Will retry strategy works for crt client? If yes can you please tell us how it actually works?

@jmklix
Copy link
Member

jmklix commented Oct 4, 2024

Yes, retry strategy should work for the s3crt client. It was added recently in this PR: #3110. So please make sure that you are using the latest version of this sdk. You can check out the pr for details of how each option is supposed to work, but for NO_RETRY it uses:

    if (strategyToUse == CrtRetryStrategyType::NO_RETRY)
    {
      aws_standard_retry_options options {};
      options.initial_bucket_capacity = 1;
      return aws_retry_strategy_new_standard(Aws::get_aws_allocator(), &options);
    }

And then that called this:

    /* standard default is 3. */
    if (!config->backoff_retry_options.max_retries) {
        config_cpy.max_retries = 3;
    }

So it looks like this might not be working correctly as I don't see max_retries get set for anything other then EXPONENTIAL_BACKOFF. Still looking into this

@jmklix jmklix added bug This issue is a bug. investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 This is a standard priority issue and removed duplicate This issue is a duplicate. guidance Question that needs advice or information. closing-soon This issue will automatically close in 4 days unless further comments are made. p3 This is a minor priority issue labels Oct 4, 2024
@jmklix jmklix self-assigned this Oct 4, 2024
@jmklix
Copy link
Member

jmklix commented Nov 27, 2024

This should be fixed with this PR. Please update to the latest version on this sdk and let us know if you see any more unexpected behavior with the retry settings

@jmklix jmklix closed this as completed Nov 27, 2024
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

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. investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

2 participants