Skip to content
This repository has been archived by the owner on Feb 22, 2019. It is now read-only.

Adding destroy method to connection pool #84

Closed
wants to merge 1 commit into from

Conversation

calvinfo
Copy link
Contributor

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:

  1. listen on pool errors
  2. close the current pool
  3. try reconnecting
pool.on('error', function () {
  pool.close();
  pool.connect([...]);
});

The problem here is that calling close on a connection tries to drain all the writes from it by calling end on 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 (via destroy) or deciding to wait on the queued data (via close)

Thoughts?

@calvinfo
Copy link
Contributor Author

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.

@devdazed
Copy link
Contributor

what about sending them off to a different connection?

@calvinfo
Copy link
Contributor Author

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.

@devdazed
Copy link
Contributor

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?

@calvinfo
Copy link
Contributor Author

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:

  1. connect to node
  2. issue a read, wait for callback
  3. CASSANDRA CRASH
  4. pool/connection emits 'error'
  5. read hangs

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

@devdazed devdazed closed this May 6, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants