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

Order of http_responses written not preserved after PSUBSCRIBE #158

Open
CapSel opened this issue May 31, 2019 · 3 comments
Open

Order of http_responses written not preserved after PSUBSCRIBE #158

CapSel opened this issue May 31, 2019 · 3 comments

Comments

@CapSel
Copy link

CapSel commented May 31, 2019

My application uses /PSUBSCRIBE and waits for notifications about keys. Redis generates hundreds of events about keys. Data that gets out of webdis to my application is "mixed" - first some http chunks are sent, then all of headers, then http chunks continues.

Fucntion http_schedule_write schedules write event for every http_response but libevent does not preserve order of calls to http_can_write. In effect first are called callbacks that sends chunks, then headers, and then there are only chunks left. Similar situation might happen when a chunk would be too long to fit with one call to write in http_can_write.

@CapSel
Copy link
Author

CapSel commented Aug 5, 2019

Since Libevent 2.0.1-alpha, any number of events may be pending for the same conditions at the same time. For example, you may have two events that will become active if a given fd becomes ready to read. The order in which their callbacks are run is undefined.

http://www.wangafu.net/~nickm/libevent-book/Ref4_event.html

@nicolasff
Copy link
Owner

Oh, that's a good find. I'll read about it but it feels like fixing this issue might require a substantial change.
What I don't understand though is why an event would be scheduled multiple times for a request: we schedule it at the end of http_response_write, and then re-schedule it from http_can_write if there's still any data left to send. Since you've looked at the code for this, have you seen where this multiple registration might be coming from?

Sorry about the delay responding to this by the way, I have not worked on Webdis for a long time.

@CapSel
Copy link
Author

CapSel commented Aug 7, 2019

So this is what I believe is happening:

  • redis is constantly sending parts of reply hitting format_send_reply()
  • format_send_reply() checks for started_responding == 0 and schedules writing of response - with headers
  • headers are not sent right away or are sent in part, the rest is queued for future
  • format_send_reply() checks for started_responding == 0 and schedules write of a chunk
  • chunk is not sent yet so it's queued
  • libevent wakes up for cmd->fd and ... messes the order

To patch it up fast I changed the line:

resp = http_response_init(cmd->w, 200, "OK");

to

resp = http_response_init(NULL, 200, "OK");

inside of code for started_responding == 0 to make this blocking call. This allowed for scheduling all json responses after headers are sent.

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

No branches or pull requests

2 participants