Excon does not work with Async

[morgoth]

I was trying to use Excon gem with async, but unfortunatelly it does not work:

require "async"
require "excon"

start = Time.now
Async do |task|
  20.times do |i|
    task.async do
      Excon.get("https://httpbin.org/delay/1.6")
      # URI.open("https://httpbin.org/delay/1.6") - works
      # Net::HTTP.get_response(URI("https://httpbin.org/delay/1.6")) - works
    end
  end
end.wait
puts Time.now - start
47.554362033

I'm wondering what decides that something works in async way.

@ioquatix Is it possible to put some light on this topic? I found it surprising after reading https://brunosutic.com/blog/async-ruby

[bruno-]

I did a quick dive into the gem source code. The gem is using non-blocking method calls in a loop:

https://github.com/excon/excon/blob/master/lib/excon/socket.rb#L179-L181

From what I can tell, this is effectively busy-waiting for the responses.

There's probably a reason behind that, but the downside is that non-blocking methods don't work with async (or more precisely: with fiber scheduler). To quote the linked article:

any blocking operation (a method where Ruby interpreter waits) is compatible with Async and will work asynchronously within Async code block with Ruby 3.0 and later.

Even nonblock: false seems to use Socket#read_nonblock for the first response chunk 🤷‍♂️

[geemus]

Hmm, author of excon chiming in, fwiw.

I don't precisely recall the reason for looping on nonblock there, other than perhaps that it worked to do what I wanted at the time. If there is a better approach you could suggest I would be open to hearing about it.

I don't think using nonblocking reads even when nonblock false is on was intended. If I recall correctly it was all blocking originally, and we then added nonblocking (and this became the default). For the most part once we started down that path, most people wanted and preferred nonblocking and so blocking became less well tested or paid attention to. I suspect this is an example of us simply overlooking this. I think the usage comes from response calling readline as it parses headers and readline using nonblocking (and not checking the setting). So it's probably pretty fixable, and I'd be open to discussing. Does that make sense?

[bruno-]

Hi

I think the usage comes from response calling readline as it parses headers and readline using nonblocking (and not checking the setting).

This is what I observed with my quick debugging yesterday.

So it's probably pretty fixable, and I'd be open to discussing. Does that make sense?

Yea, maybe try making nonblocking: false mode use only blocking methods. I think that should work nicely with fiber scheduler and async.

Also, can you avoid using the built-in selector, because I think waiting on IO.select will also block and NOT yield to fiber scheduler:

https://github.com/excon/excon/blob/aad00702f1cd59101a0759245fa7d59cc06a3d3c/lib/excon/socket.rb#L275-L287

[ioquatix]

Yes, you should avoid IO.select in your code.

You should avoid any kind of busy waiting on IO unless you also call wait_readable/writable or something similar.

[geemus]

Thanks for the suggestions, I definitely didn't (and don't) have the most experience with doing all of this correctly. We got it to the point of working and good enough for general purposes and mostly moved on. Having some tips and insights about how to improve it are therefore quite helpful (though I must admit I'm not sure when I'll be able to find time to revisit this stuff).

[bruno-]

Another idea regarding the default nonblocking mode: maybe check if Fiber.scheduler is set, and if yes, default to nonblocking: false. This way blocking behavior is the default with async and when fiber scheduler is used.

[ioquatix]

I don't advise checking fiber scheduler it should be transparent to the code. Just use the right IO operations and the rest is handled transparently.

[geemus]

Ok, I've tried to make the obvious/easy changes at least to get the readline implementation back to respecting the nonblock setting: excon/excon#782. Hopefully this solves the immediate issue. I'd welcome help in verifying that this works as expected now, as I'm not familiar with async and don't have reproduction/test code handy at present.

From some of the other comments it sounds like there might be some better ways to implement some of the other parts as well, but I was less confident about the specifics (my previous experience with IO stuff involved a lot of trial and error if I recall).

@ioquatix I'm interested in continuing to improve the code and do the waiting in a better way. Are there good examples of using wait_readable/wait_writeable properly you could recommend? I looked at the docs a little, but that really isn't the same as seeing a full example. Thanks!

[geemus]

@morgoth Oops, sorry, totally forgot that was available there.

[geemus]

Ok, I tried that out. If I run the posted version it took about 37s on my machine. Adding nonblock: false to the get call with the changes I just posted pulls it down to about 2s as expected. So looks like it did do what we wanted. I'll work on a release.

[geemus]

Ok, released in v0.92.0. As per some of the other feedback above, I think there may be more room for improvement, but we've at least fixed the biggest and most obvious issue and it looks like it should work with async (as long as you set nonblock to false, as true is the default).

[ioquatix]

My general advice is you shouldn't need any flags to make this work. If you are running in a fiber scheduler it should be non-blocking.

[geemus]

K, thanks.