cortex
cortex copied to clipboard
Send Retry-After header with 429 response or 413 if samples exceed the burst of rate limiter
What this PR does:
Which issue(s) this PR fixes:
Fixes #
Checklist
- [x] Tests updated
- [ ] Documentation added
- [x]
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]
Retry-After doesn't support duration less than one second, this is hard since we may overload our service or lose some tokens..
Hi damnever@ thank you for the PR, this is a nice change. May I ask you kindly add a CHANGELOG.md entry as well as add/update some tests? Thanks!
This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.
@alvinlin123 Somehow I have forgotten this patch. Please take another look.
@damnever Thanks for the contribution! I think this change makes sense. But for the case where the ingestion path is already overloaded then retrying could make it worse. What about making it a flag on the distributor and we can choose the behavior of sending 429 retries or 413.
@yeya24 There are two changes:
- add Retry-After header to the existing 429 responses.
- change a part of 429 response to 413 since those requests contain too many samples, retry is useless until someone raises the limit(burst).
Maybe we should add an option to enable 413 since 413 may cause the client to drop samples without retrying.
I see, thanks. If we only send 429 when it is retryable and otherwise 413 then I think it is a reasonable approach.
This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.