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

Activator: Shouldn't use http.DefaultTransport #1238

Closed
dprotaso opened this issue Jun 15, 2018 · 13 comments
Closed

Activator: Shouldn't use http.DefaultTransport #1238

dprotaso opened this issue Jun 15, 2018 · 13 comments
Assignees
Labels
area/autoscale kind/feature Well-understood/specified features, ready for coding.

Comments

@dprotaso
Copy link
Member

dprotaso commented Jun 15, 2018

transport := http.DefaultTransport

By default it doesn't have any default timeouts

See:

@google-prow-robot google-prow-robot added area/autoscale kind/feature Well-understood/specified features, ready for coding. labels Jun 15, 2018
@mattmoor
Copy link
Member

/kind good-first-issue

@erain
Copy link
Contributor

erain commented Dec 7, 2018

/assign erain

@erain
Copy link
Contributor

erain commented Dec 14, 2018

I believe we don't need to fix this issue anymore, since DefaultTransport in latest golang release already has default timeouts?

https://github.com/golang/go/blob/dbd323bb880ff27fa9b4bdfebf3e5d4828b09678/src/net/http/transport.go#L42-L53

@erain
Copy link
Contributor

erain commented Dec 17, 2018

@mattmoor could we close this issue?

@tcnghia
Copy link
Contributor

tcnghia commented Dec 17, 2018

@dprotaso do you still see a need for a different transport?

@k4leung4
Copy link
Contributor

@dprotaso any comment on this or should we close?

@dprotaso
Copy link
Member Author

dprotaso commented Feb 27, 2019

Hey sorry for the delay.

This issue is still present since the DefaultTransport doesn't set a ResponseHeaderTimeout

Thus if the server doesn't shutdown gracefully and the activator doesn't receive HTTP headers it hangs.

@hohaichi
Copy link
Contributor

@jonjohnsonjr this issue seems to be fixed by your PR#3409, right?

@jonjohnsonjr
Copy link
Contributor

Not for the activator. That PR only fixed the transport for tag -> digest resolution.

@vagababov
Copy link
Contributor

vagababov commented Apr 22, 2019 via email

@mattmoor
Copy link
Member

Fixed by: #3794

@dprotaso
Copy link
Member Author

dprotaso commented Apr 26, 2019 via email

@tcnghia
Copy link
Contributor

tcnghia commented May 3, 2019

/cc @vagababov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/autoscale kind/feature Well-understood/specified features, ready for coding.
Projects
None yet
Development

No branches or pull requests

10 participants