Hedgin Bets and Requests

[joe-elliott]

What this PR does:

Hedges GCS/S3/Azure requests using this library. This currently only supports s3 and gcs. Azure does not seem to give access to http transport so I'm not sure if we can use this solution there. This PR is ready for review but I'm still evaluating options for adding tests.
Azure support added!

Using a hedge_requests_at value of 500ms seeing the following impact in ops:

frontend latency:

• p99 from 9.8s -> 2.5s
• p50/p90 down 10s of ms
  

gcs requests/second:

• slightly elevated? almost too little to tell. this is unsurprising since 500ms is above our p99

Thx to @cristaloleg and @storozhukBM for their help in getting hedgedhttp to support GCS/HTTP2.

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[cristaloleg]

SGTM. Minor thing: 2nd param to the NewClient can be extracted to a named const, will be easier to get what 2 means.

[cristaloleg]

Made a minor change with params validation, no other changes https://github.com/cristalhq/hedgedhttp/releases/tag/v0.5.0

Also @storozhukBM suggested that hedgedhttp can be easily abstracted to any other hedging-like work, like adding same feature to Azure client even without access to http.Transport.

No sooner said than done - https://github.com/cristalhq/synx/blob/main/hedged.go but not tested and well designed yet. Will be polished next week(s).

[annanay25]

This is amazing 🚀

One more thought:

• Should we add a metric on number of hedged requests that we can track?

[joe-elliott]

@grafana/tempo Please re-review

• tests added
• azure support added

This is ready to go from my perspective.

[yvrhdn]

LGTM! 👍 Left some remarks, nothing blocking.

Also kudos for the tests, this kind of stuff is tricky to test 🙂

[yvrhdn]

Not specific to this PR: I noticed this config uses kebab-case instead of snake_case like the other configs. Is this intentional? Is this just some debt we can't get rid of anymore now?

[joe-elliott]

yeah, i noticed this for the first time adding this config option. i think everything is snake except for this azure config. we should consider a breaking change PR where we move azure to the same standard as everything else.

[storozhukBM]

@joe-elliott
FYI: in new version https://github.com/cristalhq/hedgedhttp/releases/tag/v0.6.0
We have stats that you can use for metrics

[annanay25]

Thanks @cristaloleg @storozhukBM for the super useful library and great collaboration 🚀

I agree we can move ahead with this, I'll open an issue for the metrics that we can work on in the future.