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

Restore rate limit metric distinction #5658

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

rdettai
Copy link
Collaborator

@rdettai rdettai commented Jan 29, 2025

Description

In #5651 errors where all shards are rate limited were re-qualified from no_shards_error (503) to a rate_limiting_error (429). But rate limiting errors also cover the situation where we unfortunately tried to write to rate limited shards during all 5 workbench retries. We actually need to keep the distinction between the two for troubleshooting reasons, especially in the metrics.

How was this PR tested?

Describe how you tested this PR.

@rdettai rdettai self-assigned this Jan 29, 2025
@rdettai rdettai changed the title Fix rate limit metric distinction Restore rate limit metric distinction Jan 29, 2025
@rdettai
Copy link
Collaborator Author

rdettai commented Jan 30, 2025

Moving this back to draft, because this distinction is actually not bulletproof. Having made 5 persist attempts that ended up on rate limited shards we can still not conclude whether all shards were limited or not. It's still very much function of the number of shards.

@rdettai rdettai marked this pull request as draft January 30, 2025 16:17
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.

1 participant