Add autoreconnection, fixed #25, #55

[kostya]

btw, maybe add option to reconnect or not?

Fixed

• Fixed #55, catch all possible socket/connection errors and raise Redis::ConnectionError.
• Fixed #25, when client execute command and get Redis::ConnectionError, it reconnect to redis server and reexecute last command.
• Error connection to redis server now raises Redis::CannotConnectError.
• Redis::DisconnectedError replaced with Redis::ConnectionError.
• SocketWrapper is class that wrap all methods to socket with catching all kind of errors.

test-reconnection.cr

was

"a"
"a"
"a"
Error reading socket: Connection reset by peer (Errno)
  from /usr/local/Cellar/crystal-lang/0.24.2_1/src/socket.cr:53:9 in 'unbuffered_read'
  from /usr/local/Cellar/crystal-lang/0.24.2_1/src/io/buffered.cr:206:5 in 'fill_buffer'
  from /usr/local/Cellar/crystal-lang/0.24.2_1/src/io/buffered.cr:82:7 in 'peek'
  from /usr/local/Cellar/crystal-lang/0.24.2_1/src/io.cr:322:28 in 'read_char_with_bytesize'
  from /usr/local/Cellar/crystal-lang/0.24.2_1/src/io.cr:315:12 in 'read_char'
  from src/redis/connection.cr:95:12 in 'receive'

became

"a"
"a"
"a"
#<Redis::CannotConnectError:Errno: Error connecting to 'localhost:7777': Connection refused>
"a"
"a"

test-reconnection2.cr

was

"a"
"wait for 7 seconds"
RedisError: Disconnected (Redis::DisconnectedError)
  from src/redis/connection.cr:0:7 in 'receive_line'
  from src/redis/connection.cr:96:5 in 'receive'
  from src/redis/strategy/single_statement.cr:12:5 in 'command'
  from src/redis.cr:239:5 in 'command'

became

"a"
"wait for 7 seconds"
"a"

[kostya]

simple benchmark not show any difference in performance with master:

require "./src/redis"

r = Redis.new(port: 7777)

t = Time.now
20000.times do |i|
  r.set("bla#{i}", i.to_s)
end
p Time.now - t

t = Time.now
s = 0_u64
20000.times do |i|
  if v = r.get("bla#{i}")
    s += v.bytesize
  end
end
p s
p Time.now - t

[stefanwille]

Thank you for the PR! 😄

[kostya]

there is still not solved things,

example:

r = Redis.new
r.get("bla")
sleep 500
r.pipelined do
  # ...
end

pipelined would crash by disconnected, pipeline not use reconnect, because we cannt call all blocks twice, and it need to remarshal all tasks. Multi also would not reconnect. But this is hard and rare things, i think this is ok, until we find how to fix it.

[stefanwille]

Regarding pipelined: yes, I agree.

[kostya]

may be pipeline and multi should collect all commands in buffer, not in socket, and then execute it with one command.

[stefanwille]

I believe this would be unexpected for the user. But also, the current reconnect behaviour seems fine to me with respect to. pipeline and multi.

[stefanwille]

I have added a reconnect option to the constructor.

[stefanwille]

We should be good now, no?

[stefanwille]

The reconnect option is true by default.

[stefanwille]

I will make a release later today or tomorrow, as I am running out of time at the moment.

[kostya]

i think i try add pool, and with pool and command_timeout, it can be 2.0.0