reverseproxy: fix random hangs on http/2-requests with Server Pushes #3910
Conversation
There was a problem hiding this comment.
Thanks for the PR; I'm fine with updating the net dependency but I don't think we can accept the http2.ConfigureServer() call for this.
Go tags releases fairly often -- about once per month -- so the upstream fix should be landing soon enough. I think that will be better for the stability of the project than using HEAD.
| @@ -306,6 +306,10 @@ func (app *App) Start() error { | |||
| Handler: srv, | |||
| ErrorLog: serverLogger, | |||
| } | |||
| err = http2.ConfigureServer(s, nil) | |||
There was a problem hiding this comment.
This PR is doing more than just updating the x/net module; I'm concerned that this could cause problems as it takes the stability out of our HTTP/2 stack by using the bleeding edge instead of tagged releases.
There was a problem hiding this comment.
@mholt, what about conditional usage? It could be something like that
if srv.AllowH2ExternalVersion {
...
}
There was a problem hiding this comment.
I definitely don't want to expose such an internal implementation detail to the end user to configure. This is something that really should be decided at compile-time. Our project should compile for stability, and if a user wants bleeding edge, they can compile it that way.
This is a permanent complication for a temporary problem...
I think we just need to wait until Go does a new release (just like with every other upstream Go issue), which will probably be soon. (The whole h2 vendoring thing is a weird decision anyway. Either HTTP/2 should have been in the std lib or it should stay outside, not... both... IMO.)
There was a problem hiding this comment.
OK. I agree with you.
The solution is to use explicitly recent
x/net/http2instead of bundled version, which updates only with releases of Go language.Fix: #3896
Details: golang/go#42534