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

Don't retry on HTTP 429 forever #189

Closed
breedx-nr opened this issue Jun 17, 2020 · 3 comments
Closed

Don't retry on HTTP 429 forever #189

breedx-nr opened this issue Jun 17, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@breedx-nr
Copy link
Contributor

This is a follow-up from #167 .

The fix for #167 made threads daemon, but the root problem still remains: 429 errors returned from the endpoints cause the TelemetryClient to retry forever.

There should be an upper limit to the number of consecutive retries. I'm not sure what makes a sane default, because it might be dependent on the wait-after intervals commonly returned on 429.

@breedx-nr
Copy link
Contributor Author

Because the 429 may be accompanied by a backoff time, we think that trying forever is probably the right approach. The previous change to daemon threads should also allow the JVM to be shut down cleanly, even when the 429 loop is in progress.

Feel free to reopen if you would like to revisit this.

Java Engineering Board automation moved this from To do to Done Sep 30, 2020
@breedx-nr
Copy link
Contributor Author

We should come back to this, because we will continue to buffer new telemetry during periods of 429. At some point we should probably start dropping data.

@breedx-nr breedx-nr reopened this Oct 29, 2020
Java Engineering Board automation moved this from Done to In progress Oct 29, 2020
@jodeev jodeev moved this from In progress to To do in Java Engineering Board Nov 30, 2020
@XiXiaPdx XiXiaPdx assigned XiXiaPdx and unassigned XiXiaPdx Jan 5, 2021
@XiXiaPdx
Copy link
Collaborator

XiXiaPdx commented Jan 5, 2021

429 are accompanied with a backoff time and there is also a limiting scheduler added in #251 . This issue is closed and please reopen if there is a need.

@XiXiaPdx XiXiaPdx closed this as completed Jan 5, 2021
Java Engineering Board automation moved this from To do to Done Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants