Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More connection management cleanup. #34

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aahoughton
Copy link
Contributor

Two things, which maybe ought to be split apart:

  1. node-redis explicitly clears the event observers on the underlying stream when calling client.end(); this means we'll never get an end event, and we'll never clean our clients array unless we get a sentinel failover. Work around this by explicitly filtering closing clients when creating a new client.
  2. I'm vaguely concerned about life cycle for clients that hit a handled error, for whatever reason -- it's not clear to me that we get any closing event (or even that we can reconnect). Create a concept of unmanaged connections, and slide it into the createClient options.

I'm happy to break this into different commits / remove the second, honestly. Thank you!

Andrew Houghton added 2 commits March 5, 2015 09:05
will never emit an end event when calling client.end() (crazy, right?).
Also add a concept of unmanaged connections, and test it.
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.

1 participant