search

mloughran / em-hiredis

Eventmachine redis client
MIT License
221 stars 63 forks source link

Configuring redis:// url without port defaults the port to.... zero? #27

Open dwbutler opened 10 years ago

dwbutler commented 10 years ago

Hi,

We're using Faye::Redis running in Puma under JRuby. We had an issue for quite a while where Faye communication would just.... stop.

It turns out we were using a url like redis://host without specifying the default port (6379). After diving through the code, I found that em-hiredis defaults the port to nil here and then passes the nil port along to EM.connect.

Diving into Eventmachine, it appears that it calls to_i on the port here (at least in JRuby). This means that it is trying to connect to port 0, which doesn't make a lot of sense. Furthermore, there is no exception. It appears that em-hiredis simply blocks on IO until the Puma connection times out.

I'm not sure what the correct fix would be. My gut feeling is that em-hiredis should simply default the port to 6379. I also feel that Eventmachine shouldn't accept a nil port, because a missing port can't mean anything reasonable.

Thanks, let me know what you think!

dwbutler commented 10 years ago

Some more context in case you want to reproduce... We're using Amazon's Elasticache hosted Redis. When I run the following, it just hangs forever.

require 'socket'
socket=TCPSocket.open('yourserverhere.0001.use1.cache.amazonaws.com', 0)

This might be a configuration issue. Normally I would expect to see something like this:

require 'socket'
socket=TCPSocket.open('127.0.0.1', 0)
# => Errno::ECONNREFUSED: Connection refused - connect(2)