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

Skipping responses #31

Closed
wants to merge 7 commits into from
Closed

Skipping responses #31

wants to merge 7 commits into from

Conversation

subnetmarco
Copy link

I've came across a very specific scenario where due to a high volume of write requests, calling tcpsock:receive after every operation was too expensive. In this scenario, counters on Redis were incremented and decremented in a write-and-forget fashion.

For this reason I've introduced a new skip_responses(bool_skip) function to be able to optionally instruct the client to discard every response by skipping the tcpsock:receive function call.

I've submitted this pull request just in case somebody else needs it.

@agentzh
Copy link
Member

agentzh commented Dec 5, 2013

@thefosk Your patch is wrong. You cannot leave Redis replies in the system read buffer because that way the current Redis connection will be blocked by accumulated unread data. The right approach for discarding Redis replies is to read the reply and throw it away immediately. And it's better done on the ngx_lua cosocket level.

@subnetmarco
Copy link
Author

I've updated the code to sock:receive(0) when the client is instructed to skip the responses.

@agentzh
Copy link
Member

agentzh commented Dec 6, 2013

@thefosk It's still wrong because sock:receive(0) does not consume any data in the system socket receive buffers, which could still prevent the remote redis server from sending more replies, leading to TCP connections with blocked traffic on the receiving direction.

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.

None yet

3 participants