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

The pool destroys the client with queued queries to it #632

Closed
grudzinski opened this issue Aug 20, 2014 · 6 comments
Closed

The pool destroys the client with queued queries to it #632

grudzinski opened this issue Aug 20, 2014 · 6 comments
Labels

Comments

@grudzinski
Copy link

The pool destroys the client with queued queries to it. I think the pool should reassign the queries to another client when it decided to destroy the client.

@booo
Copy link
Contributor

booo commented Aug 27, 2014

Could you please be a bit more specific? If possible provide example code. Thanks a lot.

@brianc
Copy link
Owner

brianc commented Sep 3, 2014

You should not be returning a client to the pool before it's internal queue is done draining or while it still has pending work to do. While the API doesn't specifically prevent this with an exception this it's definitely not an intended usage pattern.

@brianc brianc closed this as completed Sep 3, 2014
@grudzinski
Copy link
Author

When we call connect method on pg it returns one of client instances from its pool, then when we call query method on the client instance the query may be queued inside of the client instance if the client instance executes another query. If an error occurs in the client instance then pg destroys the client, but does not care about queued queries inside of it.

pg.connect(function(err, client, done) {

    if (err) {
        console.error(err);
        return;
    }

    client.query('SELECT 1 + 1', function(err) { // may be queued

        // #1

        // An client connection error leaks here, user's code
        // now forced to care about this pg internal error

        // How to check if it is a pg internal error or an user error (SELECT 1 / 0)?
        // Why user should care about the pg internal error?

        if (err) {
            done(err); // will cause pg to destroy the client
        }

    });

    // Now shutdown pg server while first query executing, it will
    // cause a connection error

    client.query('SELECT 2 + 2', function(err) { // will be queued

        // #2

        // the callback will not be called

    });

});

@aviramst
Copy link

aviramst commented Sep 3, 2014

Why did you call handleError of the active query if the error comes from the connection?
How can my code handle connection errors?

  con.on('error', function(error) {
    if(self.activeQuery) {
      var activeQuery = self.activeQuery;
      self.activeQuery = null;
      return activeQuery.handleError(error, con);
    }
    if(!callback) {
      return self.emit('error', error);
    }
    callback(error);
    callback = null;
  });

@brianc brianc reopened this Sep 3, 2014
@brianc
Copy link
Owner

brianc commented Sep 3, 2014

@aviramst if you're running a query & you have a callback on the query we made the decision to bubble up a connection error to the query so you can handle it in the callback. Technically your query has failed if the connection dies.

@grudzinski i see what you're saying now - valid point. Any client that's destroyed by the pool should have its query queue cleared out, certainly.

@charmander
Copy link
Collaborator

Fixed in v7.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants