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

add closenotify and large timeout to gateway #1980

Merged
merged 1 commit into from
Nov 20, 2015

Conversation

whyrusleeping
Copy link
Member

gateway requests that fail to resolve would cause goroutines to build up indefinitely.

License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
@jbenet jbenet added the status/in-progress In progress label Nov 19, 2015
@jbenet
Copy link
Member

jbenet commented Nov 20, 2015

Yep, LGTM.

Was thinking that we should have a LivenessContext that cancels if there hasn't been i/o for a while. (or of a certain rate) meaning that if a request is making forward progress, it continues. if it's not, it's killed after some timout. this will allow us to place "timeouts" on requests without hosing people with low bw/ high latency connections. I.e. if person is making fwd progress on a request that's taking an hour, the request should continue, but if the request has been totally stalled for 5m, it probably should not.

@whyrusleeping
Copy link
Member Author

I'm looking at writing that right now, and to get it as efficient as i'd want it, i would need to fork the context lib. (otherwise i need one goroutine per extra context). They (as the go team likes to do) made some methods and interfaces private that would let users efficiently extend their code.

@jbenet
Copy link
Member

jbenet commented Nov 20, 2015

:c pike!!!!.png

On Thu, Nov 19, 2015 at 10:08 PM Jeromy Johnson notifications@github.com
wrote:

I'm looking at writing that right now, and to get it as efficient as i'd
want it, i would need to fork the context lib. (otherwise i need one
goroutine per extra context). They (as the go team likes to do) made some
methods and interfaces private that would let users efficiently extend
their code.


Reply to this email directly or view it on GitHub
#1980 (comment).

@whyrusleeping
Copy link
Member Author

I'm considering actually forking it, and making the relevant parts exported. It would allow for more easily exensible contexts for anyone who wants to interop with us (or use that lib)

@jbenet
Copy link
Member

jbenet commented Nov 20, 2015

im fine with that

@whyrusleeping
Copy link
Member Author

mmmm.... forking time 💃

@whyrusleeping
Copy link
Member Author

merging, keepalive context stuff can come later.

whyrusleeping added a commit that referenced this pull request Nov 20, 2015
add closenotify and large timeout to gateway
@whyrusleeping whyrusleeping merged commit 856e250 into dev0.4.0 Nov 20, 2015
@jbenet jbenet removed the status/in-progress In progress label Nov 20, 2015
@ghost ghost deleted the fix/gateway-close-notif branch January 22, 2017 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants