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

Heroics should use keep-alive connections #16

Open
jkakar opened this issue Mar 18, 2014 · 6 comments
Open

Heroics should use keep-alive connections #16

jkakar opened this issue Mar 18, 2014 · 6 comments

Comments

@jkakar
Copy link
Contributor

jkakar commented Mar 18, 2014

This would probably give us some nice speedups.

@geemus
Copy link
Member

geemus commented Mar 19, 2014

Probably pretty straightforward to do by creating a connection on the client (and then reusing it).

@jkakar
Copy link
Contributor Author

jkakar commented Mar 19, 2014

Yep, it shouldn't be too hard. Right now connections are created in
Link.run for each request. The change we need to make is create a
single connection at the top-level Client and pass it down to links
via their parent Resource instances. It should be mostly boring
rewiring.

@schneems
Copy link
Collaborator

schneems commented Aug 27, 2020

Heroics should use keep-alive connections

I think I understand what you're suggesting here, but I want to verify it. Are you talking about making a single connection, basically opening up a socket and then re-using that socket for future requests? Is that correct?

I took a look at Link#run and see here's where we're generating a new excon connection object:

      connection = Excon.new(@root_url, thread_safe_sockets: true)

Then later we're making the request:

        connection.request(options)

Do you have any examples of using keep-alive connections with excon to follow that someone could use? Is there a way we can write an example script with keep-alive connections to quantify the speedup potential? If we had benchmarks, that show this to be a significant speedup then we could possibly prioritize this work.

@fig
Copy link

fig commented Sep 24, 2020

If I understand correctly, the assertion is that Link#run creates a new connection for each subsequent request. Considering keep-alive is the default for the majority of servers, that connection should be memoised and re-used for subsequent requests, thus providing maximum benefit by leveraging keep-alive.

@wduminy
Copy link

wduminy commented May 6, 2022

The Excon readme mentions a persistent option. Here's the example:

connection = Excon.new('http://geemus.com', :persistent => true)

Could it be that we should simply use this option? Then there would be no need to pass the connection around.

@geemus
Copy link
Member

geemus commented May 7, 2022

Hey, sorry for the long radio silence, I guess I lost track of this at some point.

To make things work as persistent, we would want to BOTH create a connection and the client level and reuse it across links (instead of creating a new connection in each link) AND set the persistent option on this new connection.

All that being said, as I recall the reason that persistent is false by default with excon is because the behavior from the server is really inconsistent and problematic. In particular, though most servers technically support keep-alive they also have a relatively short timeout on connections. Furthermore in many cases it seemed that the timeout starts when the connection is opened and did not refresh on activity. So as an example, you might have better performance for whatever requests you manage to do within the first 30 seconds of the socket, and then it would just fail. And the failure in this case can also be a race condition, ie perhaps your request got through and it timed out before responding, in that case you don't get a response because the socket is closed, but maybe the action still occurred?

I hope that makes sense, let me know if you have questions and I can try to explain further.

Taken all together, it can be a good idea to setup connections and allow an option for users to toggle persistent on if they think it is worth it and know what they are doing. ie some servers actually allow for longer keep-alive and so don't have problematic behavior, or if you know all your requests are idempotent you can allow it to fail and simply refresh the connection and retry. BUT it's probably not a reasonable default for general usage, as my experience of actually trying to use it in practice have been quite really mixed at best.

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

No branches or pull requests

5 participants