[outputs.influxdb_v2] add exponential backoff, and respect client error responses #8662
Conversation
| } | ||
| // take the highest value from both, but not over the max wait. | ||
| retry := math.Max(backoff, retryAfterHeader) | ||
| retry = math.Min(retry, defaultMaxWait) |
There was a problem hiding this comment.
I would expect telegraf to obey the Retry-After header even if it's longer than defaultMaxWait. If the backend is in trouble and ops needs to quiet down retries, they will increase Retry-After and telegraf should obey it.
There was a problem hiding this comment.
yeah, agreed. I had checked with the db team, and all of their use cases either don't use it (OSS 2.x doesn't use it at all), or it's only used as rate limiting. I suspect it would be prudent to still have a maximum as I wouldn't really want it to sit idle for 2 hours due to an expiry bug. I think we should set defaultMaxWait to whatever we're comfortable with as a max wait time and leave it at that. I can't really see anything above 60s being hugely valuable, but it seems a lot more likely to cause problems.
I'm proposing upping the defaultMaxWait to 60s. let me know what you think.
There was a problem hiding this comment.
I prefer trusting the header unconditionally but if you don't want to, it's not a big deal for me because the situation we're talking about is unlikely anyway. Increasing absolute max is the next best thing, whether it's 1 or 5 or 10 minutes.
|
!signed-cla |
![telegraf-tiger[bot]](https://avatars.githubusercontent.com/in/94907?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla
|
!signed-cla |
| } | ||
| // take the highest value from both, but not over the max wait. | ||
| retry := math.Max(backoff, retryAfterHeader) | ||
| retry = math.Min(retry, defaultMaxWait) |
There was a problem hiding this comment.
I prefer trusting the header unconditionally but if you don't want to, it's not a big deal for me because the situation we're talking about is unlikely anyway. Increasing absolute max is the next best thing, whether it's 1 or 5 or 10 minutes.
srebhan
added
area/influxdb
feat
…or responses (influxdata#8662) * [outputs.influxdb_v2] add exponential backoff, and respect client error responses * add test * Update to 60 seconds * fix test
resolves #8571