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

Selecting a database on re-connect - discussion #86

Closed
cloudhead opened this Issue Apr 6, 2011 · 4 comments

Comments

Projects
None yet
7 participants
@cloudhead

cloudhead commented Apr 6, 2011

I'd like to have an opinion on this issue:

I use different clients for different databases, and so I select the database I want when the client is first created. Sometimes, connections get dropped, and so the selected database is no longer selected on reconnect. Of course, I could call select before each command, but that seems like a huge waste. So ideally, the command would be run every time the client connects.

Because of the way node_redis works though, commands sent from the "connect" event listener will be sent after commands which were sent while offline, this is because of the offline_queue, and the fact that ready is set after "connect" is emitted, so commands in the listener are queued!

The "ready" event is then emitted after all this, but at that point the offline commands are already sent, with the wrong database selected.

The solution I'm currently using, which seems to work is:

  client.on('connect', function () {
      client.send_anyway = true;
      client.select(DB);
      client.send_anyway = false;
  });

This makes it so the select skips the ready-check, and sends the command before the offline_queue is sent.

Here's where this is all happening:

https://github.com/mranney/node_redis/blob/master/index.js#L187

A nice solution would be to emit "connect" after ready = true. Another solution would be to have another event there, like "connected", which gets emitted after we're ready, but before the queue is sent.

Thoughts?

@tisba

This comment has been minimized.

Show comment
Hide comment
@tisba

tisba May 26, 2011

In fact, currently you need to select the db on the error event. Otherwise you risk that your select command get's queued behind other commands in the offline_queue which is reseted while handling error (see e.g. https://github.com/mranney/node_redis/blob/master/index.js#L108). It took me a while to figure out what's the best way to ensure that you only write to a specific db.

I think there should be way to insert a command into the offline queue, before all other events.

tisba commented May 26, 2011

In fact, currently you need to select the db on the error event. Otherwise you risk that your select command get's queued behind other commands in the offline_queue which is reseted while handling error (see e.g. https://github.com/mranney/node_redis/blob/master/index.js#L108). It took me a while to figure out what's the best way to ensure that you only write to a specific db.

I think there should be way to insert a command into the offline queue, before all other events.

@ciaranj

This comment has been minimized.

Show comment
Hide comment
@ciaranj

ciaranj Jul 31, 2011

Hmm, this has also just bitten me. Are there any other stateful-persistent commands that can be sent to a redis server? .. I, like you @cloudhead use the same pattern of 1 client per DB .. and it took me an age to work out why my data wasn't coming back! :)

ciaranj commented Jul 31, 2011

Hmm, this has also just bitten me. Are there any other stateful-persistent commands that can be sent to a redis server? .. I, like you @cloudhead use the same pattern of 1 client per DB .. and it took me an age to work out why my data wasn't coming back! :)

@mranney

This comment has been minimized.

Show comment
Hide comment
@mranney

mranney Nov 15, 2011

Contributor

Hey guys, we still need to fix this for subscribe events, but I think the db re-selection issue is fixed in v0.7.0.

@cloudhead, can you see if this fixes your issue?

Contributor

mranney commented Nov 15, 2011

Hey guys, we still need to fix this for subscribe events, but I think the db re-selection issue is fixed in v0.7.0.

@cloudhead, can you see if this fixes your issue?

@maritz

This comment has been minimized.

Show comment
Hide comment
@maritz

maritz Dec 6, 2011

It's fixed for me. (ran into the same issue and thought I was already using v0.7.1, turns out I didn't)

Now, the other question is why I'm constantly losing my redis connection?! :/

maritz commented Dec 6, 2011

It's fixed for me. (ran into the same issue and thought I was already using v0.7.1, turns out I didn't)

Now, the other question is why I'm constantly losing my redis connection?! :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment