segmentio / nsq.js

NSQ client for nodejs
203 stars 34 forks source link

don't emit finish till server recieves FIN #44

Closed gjohnson closed 10 years ago

gjohnson commented 10 years ago

I think in some cases, like when gracefully closing a process, since we emit finish at the same time as sending FIN and we may be exiting the process before the FIN actually got sent over the wire.

Doesn't sound like there is a response back from the FIN though, so not sure how we can easily know the writes got flushed. Perhaps setImmediate?

https://github.com/segmentio/nsq.js/blob/master/lib/connection.js#L325-L326

CC: @calvinfo @visionmedia

mreiferson commented 10 years ago

This is unfortunate, sorry (not getting a response back from FIN) :grin:

I have a suggestion though: what about tracking outstanding messages and waiting for that to hit 0 before allowing the process to exit?

I don't think it's as important to "delay" the actual FIN event vs. ensuring that you'v written/flushed the data to the socket before exiting (best effort).

tj commented 10 years ago

Hmmm I wonder how we can sneak the .write callback in without being super derpy. We have the .command() callback already, so we might have to pass another one I guess haha, or only pass it on the !hasResponseCallback() check maybe

gjohnson commented 10 years ago

Yeah something like that @visionmedia, I'll take a look when we need to refactor other things.

@mreiferson we actually already do that. The problem is that were cheating and not providing callbacks to writes() since you can rely on error events to detect most issues. However when we write and then immediately decrement the counters, in the case of closing the process, it will think everything is safe to close because the counter is zero, but the write() may not have finished quite yet. YAY async!

Really only a problem on a super busy reader, but we have a couple of those :-)

mreiferson commented 10 years ago

Ah, I see, cool.

What about just force-flushing the socket (and waiting for that callback to return) before you finally exit (after the outstanding message counter has hit 0)... is that possible?

gjohnson commented 10 years ago

Think so, probably plays into the socket close event somewhere. Good call!