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

Retry logic for S3 bucket client #5135

Merged
merged 14 commits into from
Feb 28, 2023
Merged

Conversation

alexqyle
Copy link
Contributor

@alexqyle alexqyle commented Feb 9, 2023

What this PR does:

  • Created S3 bucket client wrapper to have retry logic for any bucket client failures. Retry could be configured.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

… handling

Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
docs/blocks-storage/querier.md Outdated Show resolved Hide resolved
pkg/storage/bucket/s3/bucket_client.go Outdated Show resolved Hide resolved
pkg/compactor/shuffle_sharding_planner.go Outdated Show resolved Hide resolved
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
@alexqyle alexqyle changed the title Retry logic for S3 bucket client and visit marker error handling Retry logic for S3 bucket client Feb 27, 2023
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Copy link
Collaborator

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this change looks good to me. Thanks!
Probably 2 min is a little bit too much for max retry as usually a single query timeout is < 2m.

Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
@yeya24 yeya24 merged commit d9faee1 into cortexproject:master Feb 28, 2023
@alexqyle alexqyle deleted the s3-retry branch March 21, 2023 17:59
alexqyle added a commit to alexqyle/cortex that referenced this pull request May 2, 2023
* Added retry logic for S3 bucket client and updated visit marker error handling

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Updated CHANGELOG

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Updated docs

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Changed default max retry

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Update doc

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Revert not related bug fix from s3 retry logic change

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Hard coded S3 retry config

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Revert unneccessary change

Signed-off-by: Alex Le <leqiyue@amazon.com>

* clean ws

Signed-off-by: Alex Le <leqiyue@amazon.com>

* Revised retry config

Signed-off-by: Alex Le <leqiyue@amazon.com>

* trigger workflow

Signed-off-by: Alex Le <leqiyue@amazon.com>

* trigger workflow

Signed-off-by: Alex Le <leqiyue@amazon.com>

---------

Signed-off-by: Alex Le <leqiyue@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants