-
Notifications
You must be signed in to change notification settings - Fork 525
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
output: Retry document-level 429
s by default
#13620
output: Retry document-level 429
s by default
#13620
Conversation
Updates the APM Server to automatically retry document-level `429`s from Elasticsearch to avoid dropping data. It can be configured/overwritten by `output.elasticsearch.max_retries`, and defaults to `3`. It uses the default backoff configuration, which could wait up to 1m if enough retries are configured, but can be overwritten as well. Signed-off-by: Marc Lopez Rubio <[email protected]>
This pull request does not have a backport label. Could you fix it @marclop? 🙏
NOTE: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retrying on document level 429 is the default if RetryOnDcoumentStatus is empty: https://github.com/elastic/go-docappender/blob/main/config.go#L76
@carsonip yes, I'm aware. I wanted to make it explicit to avoid any potential breaking changes going forward. It's also clearer when reading the code what will happen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought this PR is a no-op, but I was wrong. MaxDocumentRetries
was 0 and it is now non-zero. Changes lgtm but it will need a changelog.
Signed-off-by: Marc Lopez Rubio <[email protected]>
Co-authored-by: Carson Ip <[email protected]>
Updates the APM Server to automatically retry document-level `429`s from Elasticsearch to avoid dropping data. It can be configured/overwritten by `output.elasticsearch.max_retries`, and defaults to `3`. It uses the default backoff configuration, which could wait up to 1m if enough retries are configured, but can be overwritten as well. --------- Signed-off-by: Marc Lopez Rubio <[email protected]> Co-authored-by: Carson Ip <[email protected]> (cherry picked from commit 28436dc) # Conflicts: # changelogs/head.asciidoc
… (#13621) * output: Retry document-level `429`s by default (#13620) Updates the APM Server to automatically retry document-level `429`s from Elasticsearch to avoid dropping data. It can be configured/overwritten by `output.elasticsearch.max_retries`, and defaults to `3`. It uses the default backoff configuration, which could wait up to 1m if enough retries are configured, but can be overwritten as well. --------- Signed-off-by: Marc Lopez Rubio <[email protected]> Co-authored-by: Carson Ip <[email protected]> (cherry picked from commit 28436dc) # Conflicts: # changelogs/head.asciidoc * Fix conflicts, update changelog Signed-off-by: Marc Lopez Rubio <[email protected]> --------- Signed-off-by: Marc Lopez Rubio <[email protected]> Co-authored-by: Marc Lopez Rubio <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Motivation/summary
Updates the APM Server to automatically retry document-level
429
s from Elasticsearch to avoid dropping data. It can be configured/overwritten byoutput.elasticsearch.max_retries
, and defaults to3
.It uses the default backoff configuration, which could wait up to 1m if enough retries are configured, but can be overwritten as well.
This reduces data loss whenever Elasticsearch is overhelmed and reduces the chances of data loss due to ES push-back.
Checklist
- [ ] Documentation has been updatedHow to test these changes
This may need a bit of setup. It's thoroughly tested in the
go-docappender
, but we may want to verify that retries happen when document-level 429s happen.I'd suggest writing a small "proxy" that returns 429s randomly and test that things are indexed (as duplicates).
Related issues
Closes #13619