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

Fix empty waiting_queue warning #103

Merged
merged 3 commits into from
Mar 10, 2017
Merged

Conversation

doyoubi
Copy link
Contributor

@doyoubi doyoubi commented Jan 5, 2017

The Problem

There are lots of "server is readable but waiting_queue is empty" warnings in our logs when connections are closed by redis. This log was mainly used to find whether this case exists: we didn't send any command to redis but still receive response data. However, now this warning arose just because we didn't check whether the readable event is to close connection.
Most of the time this will not do harm. But in a rare case this will result in "-ERR Proxy error\r\n" response. Here's how it happen:
(1) Client send command to corvus
(2) corvus save this command in ready_queue of the connection between corvus and redis and wait for the writable event to forward this command to redis.
(3) Before corvus send the command, redis close the connection because of timeout.
(4) corvus receive readable event from redis but find its waiting_queue is empty. Then it send the Proxy error response to client.

How To Solve It

Distinguishing whether the readable event is to close connection is the perfect solution, but this require changing many codes so I think it's not worth it.
An easy workaround is to let corvus always close the connection before redis.
Firstly, use a server_timeout in corvus config which is smaller than the timeout in redis config, for example 500 for server_timeout and 600 for timeout. The gap between these two timeouts should be big enough to prevent the case that when corvus is about to send request to redis, redis close the connection at the same time.
Secondly, instead of the timeout of command request to redis, change server_timeout to be the connection timeout. This is exactly what this PR do.

@doyoubi doyoubi changed the title Fix empty waiting_queue warning [WIP] Fix empty waiting_queue warning Jan 5, 2017
@doyoubi doyoubi changed the title [WIP] Fix empty waiting_queue warning Fix empty waiting_queue warning Feb 28, 2017
@doyoubi doyoubi merged commit 92dfe53 into eleme:master Mar 10, 2017
@doyoubi doyoubi mentioned this pull request May 9, 2017
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