Bug of reconnecting

[metellica]

Hi, in the past few days, we were facing an issue if the connection between the node_redis and the redis is down and reconnect immediately, the node_redis will resend some commands(which has been failed by the disconnection of the previous connection) to the server again, but unfortunately, the node_redis seems to never expect these command's response.

For example, it send A, B, C, D to the redis just before the connection is down, and when the connection is down, it do the callback of A,B,C,D with error, after reconnecting, it send A,B,C,D again, and then send E,F,G,H, the redis responses by the requesting sequence:A,B,C,D,E,F,G,H, but the node_redis callback E with A's response, callback F with B's response, and so on. In this case, the application layer will be messed up.

We dig into the codes and found out the root cause, when reconnecting, the node_redis will just call stream.connect() without clearing the data buffer in the stream, so when the new connection is established, it will send the old data again but in the new connection's command queue, those commands never exists.

We have fixed it by creating a new stream object like this, we are not sure if it's the best way or not, but at least, it works. Please fix it, and we will be appreciated if you tell us some better way.

RedisClient.prototype.connection_gone = function (why) {
//......
//self.stream.connect(self.port, self.host);
self.stream.destroy();
self.stream = net.createConnection(self.port, self.host);
self.stream.on("connect", function () {
self.on_connect();
});

self.stream.on("data", function (buffer_from_socket) {
    self.on_data(buffer_from_socket);
});

self.stream.on("error", function (msg) {
    self.on_error(msg.message);
});

self.stream.on("close", function () {
    self.connection_gone("close");
});

self.stream.on("end", function () {
    self.connection_gone("end");
});

self.stream.on("drain", function () {
    self.should_buffer = false;
    self.emit("drain");
});

self.retry_timer = null;

}, this.retry_delay);

[chang290]

yes, net.socket has a write buffer。
If just call "connect" function,it will not clear the buffer.

[novemberborn]

Since 0.10.26 the socket doesn't emit new error events (<nodejs/node-v0.x-archive@dee5270), so reusing it like the driver does currently leads to unexpected behavior.

[brycebaril]

The reconnection logic has a number of issues in addition to this, it is why I am planning on separating the reconnection logic from this library.

[blainsmith]

In order to better support this project and its new group of collaborators under a new org we are trying to clean up and close issues older than 6 months. Your issue may have been fixed since the issue was open, however, if you are still experiencing a problem please re-open this issue and we will label it accordingly.

[BridgeAR]

@metellica version 2 should have fixed all reconnection issues. If you still encounter any of these, please give me a short reply. And it would be great to be on the safe side that everything works as it should.

[metellica]

Ruben, thanks for your notification, we will test with version 2. Any questions will let you know.

—
通过 Mailbox 发送

On Sat, Sep 26, 2015 at 1:28 AM, Ruben Bridgewater
notifications@github.com wrote:

@metellica version 2 should have fixed all reconnection issues. If you still encounter any of these, please give me a short reply. And it would be great to be on the safe side that everything works as it should.

Reply to this email directly or view it on GitHub:
#601 (comment)