-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[WIP] fix: http probe downgrades from http2 to http1 when it encounters an error #15436
base: main
Are you sure you want to change the base?
Conversation
…error Signed-off-by: Calum Murray <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Cali0707 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cc @dprotaso I made a start here, but I think I am still missing something... Specifically, if the error on the options request is because the service isn't ready yet we should try the options request again. However, I wasn't sure the best way to do that and/or figure that out. Maybe it makes sense to track how many times the options request fails before we actually set the HTTP version on the probe config? That way, it would retry the options request a few times WDYT? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15436 +/- ##
==========================================
+ Coverage 84.54% 84.66% +0.11%
==========================================
Files 219 219
Lines 13595 13600 +5
==========================================
+ Hits 11494 11514 +20
+ Misses 1734 1710 -24
- Partials 367 376 +9 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Calum Murray <[email protected]>
I've tried to update it so that until a probe succeeds for either http1 or http2, we keep retrying the options request. This seems better, but there is still a chance of a race condition where:
|
This Pull Request is stale because it has been open for 90 days with |
Fixes #15432
Proposed Changes
Release Note