Hard-coded short open timeout hurts when pull-through caching on high-latency ISP

robbkidd commented 5 months ago

Okay, here's the situation.

We're using gemstash to reduce the load on sparse internet this weekend at Ruby for Good. Our internet connection has some pretty high latency. gemstash's missing-in-cache-so-fetch-it activity would often run afoul of the 2-second hard-coded timeout for gemstash to open a connection to the configured upstream.

My parents went away on a week's vacation.

I whittled the replication case down to the following run while on our sparse internet connection:

root@gemstash:/usr/src/app# irb
irb(main):001:0> require 'faraday'
=> true
irb(main):002:0> require "faraday_middleware"
=> true

Just enough HTTPClient.for() …

irb(main):014:1* client = Faraday.new('https://rubygems.org') do |config|
irb(main):015:1*     config.use FaradayMiddleware::FollowRedirects
irb(main):016:1*     config.adapter :net_http
irb(main):017:1*     config.options.timeout = 10 # what we're using for the configurable :fetch_timeout
irb(main):018:0> end
=> 
#<Faraday::Connection:0x0000007fa2fb2f80                                                                            
...

Just enough client.get() …

irb(main):019:0> client.get('/gems/rails-7.1.3.3') { |req| req.options.open_timeout = 2 }
/usr/local/lib/ruby/3.2.0/net/http.rb:1271:in `initialize': Failed to open TCP connection to rubygems.org:443 (execution expired) (Faraday::ConnectionFailed)
… 😭

irb(main):020:0> client.get('/gems/rails-7.1.3.3') { |req| req.options.open_timeout = 10 }
=> 
#<Faraday::Response:0x0000007fa2879c50
… ✅

They left the keys to the brand new Porsche

So, I've forked and hard-coded a larger timeout for the moment.

Would they mind?

Would the gemstash project be up for the open_timeout becoming configurable like the fetch timeout? I'm happy to start a PR.

… with some guidance on the nuances of the implementation. For my temporary hack, I first attempted to naively reuse :fetch_timeout and it became clear that the gemstash_env utility is currently only mixed in as a class method (extend Gemstash::Env::Helper), not instance (include Gemstash::Env::Helper). If this open_timeout becomes configurable, I don't know if you would prefer mixing in as instance methods, too, or passing the timeout as a new parameter to HTTPClient.new(), or some other means I haven't thought of.

indirect commented 5 months ago

Thanks for figuring this out!

Would the gemstash project be up for the open_timeout becoming configurable like the fetch timeout? I'm happy to start a PR.

Yes, absolutely, go for it.

robbkidd commented 5 months ago

Opened #387! 💞

rubygems / gemstash