Skip to content

Commit

Permalink
Automatically reconnect after fork regardless of reconnect_attempts
Browse files Browse the repository at this point in the history
When an inherited connection is detected, we used to raise a BaseConnectionError
so an automatic reconnection would happen if `reconnect_attempts > 0`.

But it makes more sense to just reconnect, as it's not really an error case.
  • Loading branch information
byroot committed Feb 10, 2023
1 parent d0d7355 commit 4f5c059
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 28 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

* Automatically reconnect after fork regardless of `reconnect_attempts`

# 4.8.0

* Introduce `sadd?` and `srem?` as boolean returning versions of `sadd` and `srem`.
Expand Down
13 changes: 2 additions & 11 deletions lib/redis/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -399,23 +399,14 @@ def establish_connection
end

def ensure_connected
disconnect if @pending_reads > 0
disconnect if @pending_reads > 0 || (@pid != Process.pid && !inherit_socket?)

attempts = 0

begin
attempts += 1

if connected?
unless inherit_socket? || Process.pid == @pid
raise InheritedError,
"Tried to use a connection from a child process without reconnecting. " \
"You need to reconnect to Redis after forking " \
"or set :inherit_socket to true."
end
else
connect
end
connect unless connected?

yield
rescue BaseConnectionError
Expand Down
26 changes: 9 additions & 17 deletions test/redis/fork_safety_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,18 @@ class TestForkSafety < Minitest::Test
driver(:ruby, :hiredis) do
def test_fork_safety
redis = Redis.new(OPTIONS)
redis.set "foo", 1
assert_equal "PONG", @redis.ping

child_pid = fork do
begin
# InheritedError triggers a reconnect,
# so we need to disable reconnects to force
# the exception bubble up
redis.without_reconnect do
redis.set "foo", 2
end
exit! 0
rescue Redis::InheritedError
exit! 127
pid = fork do
1000.times do
assert_equal "OK", @redis.set("key", "foo")
end
end

_, status = Process.wait2(child_pid)

assert_equal 127, status.exitstatus
assert_equal "1", redis.get("foo")
1000.times do
assert_equal "PONG", @redis.ping
end
_, status = Process.wait2(pid)
assert_predicate(status, :success?)
rescue NotImplementedError => error
raise unless error.message =~ /fork is not available/
end
Expand Down

0 comments on commit 4f5c059

Please sign in to comment.