nicolasff / webdis

A Redis HTTP interface with JSON output
https://webd.is
BSD 2-Clause "Simplified" License
2.84k stars 304 forks source link

Order of http_responses written not preserved after PSUBSCRIBE #158

Open CapSel opened 5 years ago

CapSel commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

So this is what I believe is happening:

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.