Adding destroy method to connection pool#84
Adding destroy method to connection pool#84calvinfo wants to merge 1 commit intosimplereach:masterfrom
Conversation
|
Looking at this more, it also doesn't quite solve my problem. I want the callbacks to error out on the individual requests when destroy is called on the connection, which doesn't seem to happen. |
|
what about sending them off to a different connection? |
|
Sure, I don't have problems with the reconnecting exactly whether I use a new connection or the same connection. The main thing I want is to have some means of 'canceling' queued writes by calling them back. If an error occurs, whether I call destroy or close, the calls will continue to hang. |
|
I meant, destroying the current connection and sending the request to a different node. Or do you think this is best to be left up to the client to decide? |
|
Ah, I see. I think that's also good for the client to decide. I'm still not sure how to fix my problem of the hanging callbacks, the sequence of events would be:
At this point, I'd like some way to signal that queued read that it should stop waiting. I was hoping 'destroy' would accomplish this by discarding the queued writes and calling back with an error, but it doesn't seem to. I think do think pool and connection level destroy methods would be good things to add so we don't have to reach into the internal connections destroy method: https://github.com/simplereach/helenus/blob/master/lib/connection.js#L244 |
Note: this will only pass when run with the helenus-thrift version I've pull requested: simplereach/node-thrift#6
I've recently been trying to handle connection errors in the case of a node going down in the middle of a write.
Unless I'm missing something, my best course of action is to:
The problem here is that calling
closeon a connection tries to drain all the writes from it by callingendon its internal connection. See the net connection .end() and the stream .end() vs using destroy. This causes all my queued writes to hang for a node that usually isn't coming up anytime soon. The client should be able to choose between discarding those writes for the sake of returning quickly (viadestroy) or deciding to wait on the queued data (viaclose)Thoughts?