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

Distributor: add config flag to control error code when rate limit reached #5752

Merged
merged 9 commits into from
Aug 17, 2023

Conversation

ying-jeanne
Copy link
Contributor

@ying-jeanne ying-jeanne commented Aug 15, 2023

Distributor: add config flag to control error code when rate limit reached

What this PR does

Introducing a dedicated flag for this specific cell/tenant. When activated, it ensures a 503 response instead of a 429 under specific conditions. This facilitates client retries, minimizing data loss during pushes.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

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

@ying-jeanne ying-jeanne force-pushed the addflag branch 2 times, most recently from 30a5194 to 0887a68 Compare August 15, 2023 12:58
@ying-jeanne ying-jeanne marked this pull request as ready for review August 15, 2023 13:00
@ying-jeanne ying-jeanne requested a review from a team as a code owner August 15, 2023 13:00
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Please add this new per-tenant option to the list of experimental flags in about-versioning.md document.

@ying-jeanne ying-jeanne force-pushed the addflag branch 2 times, most recently from 39e25c8 to 7a17b76 Compare August 16, 2023 08:31
@ying-jeanne ying-jeanne requested a review from a team as a code owner August 16, 2023 08:31
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Good work, left some minor suggestions.

pkg/distributor/distributor.go Show resolved Hide resolved
pkg/distributor/distributor.go Outdated Show resolved Hide resolved
pkg/distributor/distributor_test.go Outdated Show resolved Hide resolved
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Thanks. Left few more nits, but lgtm otherwise.

CHANGELOG.md Outdated Show resolved Hide resolved
pkg/distributor/distributor.go Outdated Show resolved Hide resolved
docs/sources/mimir/configure/about-versioning.md Outdated Show resolved Hide resolved
pkg/util/validation/limits.go Outdated Show resolved Hide resolved
ying-jeanne and others added 4 commits August 17, 2023 14:00
Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
@pstibrany pstibrany enabled auto-merge (squash) August 17, 2023 12:21
@pstibrany pstibrany merged commit df45dbe into main Aug 17, 2023
28 checks passed
@pstibrany pstibrany deleted the addflag branch August 17, 2023 12:30
pstibrany added a commit that referenced this pull request Aug 30, 2023
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
pstibrany added a commit that referenced this pull request Sep 7, 2023
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
pstibrany added a commit that referenced this pull request Sep 7, 2023
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants