Skip to content

client: make Redis::Client#ensure_connected handle fork reconnects#414

Merged
pietern merged 1 commit intoredis:masterfrom
yaauie:fork-child-auto-reconnect
Apr 2, 2014
Merged

client: make Redis::Client#ensure_connected handle fork reconnects#414
pietern merged 1 commit intoredis:masterfrom
yaauie:fork-child-auto-reconnect

Conversation

@yaauie
Copy link
Contributor

@yaauie yaauie commented Mar 14, 2014

If @reconnect is enabled and #ensure_connected is called in the forked child of
the process that established the connection, a reconnect attempt should be made;
handle this case like any other ConnectionError.

If reconnect is enabled and ensure_connected is called in the forked child of
the process that established the connection, a reconnect attempt should be made;
handle this case like any other ConnectionError.
@saizai
Copy link

saizai commented Mar 14, 2014

+1

@saizai
Copy link

saizai commented Mar 14, 2014

See also #389 and #364.

@yaauie
Copy link
Contributor Author

yaauie commented Mar 14, 2014

It looks like the build failure is unrelated, and only on jruby-19mode conn=ruby REDIS_BRANCH=2.8:

 1) Failure:
test_resolves_localhost(TestInternals) [/home/travis/build/redis/redis-rb/test/internals_test.rb:347]:
Exception raised:
<#<Redis::CannotConnectError: Timed out connecting to Redis on localhost:6381>>.

@yaauie
Copy link
Contributor Author

yaauie commented Apr 2, 2014

Can I do anything to help this move forward?

@Bertg
Copy link

Bertg commented Apr 2, 2014

I was wondering if @pietern is not moving on this because of the work he is doing with the hiredis project.
Maybe this type of patch is not compatible with it; and he'd like to keep them in sync?

@pietern
Copy link
Collaborator

pietern commented Apr 2, 2014

@Bertg This is compatible. The functionality in redis-rb is just a superset of the functionality in hiredis-rb.

@yaauie Thanks for this patch. It's the most elegant one I've seen so far to address this problem. The test failure you have seen is a flaky one. The JRuby ones fail every now and then, usually related to I/O errors or timeouts.

pietern added a commit that referenced this pull request Apr 2, 2014
client: make Redis::Client#ensure_connected handle fork reconnects
@pietern pietern merged commit 443873f into redis:master Apr 2, 2014
@Bertg
Copy link

Bertg commented Apr 2, 2014

Sweet. Thx @pietern and @yaauie.
Any indication when this will be released as a gem?

@yaauie yaauie deleted the fork-child-auto-reconnect branch April 2, 2014 20:44
@rrauenza
Copy link

+1 on the gem -- can we get a 3.0.8?

@yaauie
Copy link
Contributor Author

yaauie commented May 1, 2014

If there's anything I can do to assist in getting this into a release (any other bugs that need squashing or features that need completion/verification), I'd be more than willing to lend help.

@eljuanchosf
Copy link

Guys, we are in desperate need of a 3.0.8 release with this fix. How can we help?

@yaauie yaauie added this to the v3.1 milestone Jun 4, 2014
@yaauie
Copy link
Contributor Author

yaauie commented Jun 6, 2014

This feature was released in 3.1.0.

oswellmadya pushed a commit to oswellmadya/canvas-lms that referenced this pull request May 1, 2025
See redis/redis-rb#414

This allows canvas to work cleanly with redis on newer passenger versions
which explicitly establish an activerecord connection before calling
starting_worker_process, since switchman also establishes a redis connection
and thus would generate a bunch of noise from trying to reuse a forked
redis connection.

Change-Id: I228cc6717362693892c36ca6616ccba15aaff2fc
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/255631
Reviewed-by: Ethan Vizitei <evizitei@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants