-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Question: How to properly handle unexpected disconnection / autoreconnect ? #1324
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
Comments
Hey @abenhamdine - I'm glad to see the patch I put into Before when connections unexpectedly dropped from the backend or a network partition happened node-postgres would silently ignore the closed connection and not always purge the disconnected connections from the pool so your pool could fill up with stale connections under heavy load. Under light load connections are purged on a configurable timeout, so you like wouldn't see the old 'stale connection' issue unless you had sustained heavy load for many days in production or were doing db failovers or the like. What's happening now is one (or many) client(s) in your pool sits idle and connected to the backend database. A network partition happens or database failure, fail-over, forced disconnection from the backend, etc and the idle client(s) all notice and emit an error event. The pool listens for errors on idle clients. When an idle client has an error the pool destroys the client and removes it from the pool so the next time you request a client from the pool it will automatically create and connect a new client. It's unlikely the error is being emitted from the pool during an active query (if so, this is a bug) - during query execution a client will delegate error handling to its active query so the query can call it's callback with the error (or reject it's promise, etc). You'll want to put a tl; dr - const pool = new Pool()
pool.on('error', (err) => {
console.error('An idle client has experienced an error', err.stack)
}) Hope this helps. I've tagged this as a doc candidate so I'll refine the docs here & make an entry in the docs about error handling of pooled clients. |
Also @abenhamdine I know this isn't really the best place for this kinda conversation but I'll be in Paris in september from the 21st through the 30th if you want to meet up for a coffee or something & chat about node-postgres. If so, email me & we can plan! [email protected] - if not, no worries! |
Thx you so much for such a complete & precise explanation ! 👍 💃 The 7.0 release (and its docs !) looks very promising, so good luck ! Of course, i will be glad to meet you in Paris late september 🍷 |
Hi @brianc, I faced this issue in my application. Thank you for your explanation I was able to put in the console.error and know why and when it is happening. My question is how should I handle it in the pool.on ('error', () => { }) ? I mean to say, how would I reconnect automatically when the connection terminates ? At this point, I'm required to restart the node server manually when the connection is terminated. Please pardon if it is a silly question, just wanted to know what would be the best way to do this ? |
@vyerneni17 attempting to belatedly provide an answer for @brianc: You don't need to do anything in the listener aside logging it to your destination of choice.
You still want to add a listener:
Hope that helps! |
I'm encountering problems on this front as well. I'm trying to intercept momentary interruptions (e.g., due to a db restart) which emit this error: Initially I tried connecting a handler for pool errors on the const pool = new pg.Pool({});
pool.on('error', err => console.log(err)); I then tried waiting to attach the error handler until I get the const pool = new pg.Pool({});
pool.on('connect', () => {
pool.on('error', err => console.log(err));
}); Much better luck there. I got the errors logged out. Unfortunately somewhere along the way there is still an error thrown from the connection itself: [exiting] Fatal exception: error: terminating connection due to unexpected postmaster exit
at Connection.parseE (/opt/runtime/node_modules/pg/lib/connection.js:553:11)
at Connection.parseMessage (/opt/runtime/node_modules/pg/lib/connection.js:378:19)
... @brianc or others: Can you advise on a thorough, proper implementation which will avoid unhandled errors in situations like these? Have searched the issues, but haven't had much luck for a comprehensive solution with the new pools. |
I'm very sorry to add to the confusion here. It just occurred to me that I might have another pool (aside from the one I was working with above) in place, and indeed that's the pool that was throwing unhandled errors. The simplest solution appears to work great for my case. Thanks! |
@brianc @pcothenet I'm running my node process with nodemon, I'm not sure if this changes things, but when this error occurs it stops the process.
(I've already added the event listener) |
@CoreyCole How did you add the event listener? |
import { Injectable } from '@nestjs/common';
import { Client, QueryResult, Pool } from 'pg';
import { EnvironmentsService } from '../environments.service';
@Injectable()
export class PostgresRepositoryService {
private client: Client;
private pool: Pool;
constructor(private environment: EnvironmentsService) {
this.client = new Client({
host: this.environment.getDatabaseUrl(),
port: this.environment.getDatabasePort(),
database: 'db_name',
user: 'db_user',
password: this.environment.getDatabasePassword()
});
this.client.connect()
.catch(err => {
console.error(`[PostgresRepositoryService][constructor]: error connecting to db [${err}]`);
process.exit(1);
});
this.pool = new Pool();
this.pool.on('error', (err) => {
console.error('An idle client has experienced an error', err.stack);
});
}
// other code |
@CoreyCole In that code, the pool has an error listener, but the client doesn’t. (Did you mean to create a separate pool and client at all?) |
@charmander I don't understand what the pool is, honestly. I just added the error listener on a new pool to try to make my nodemon server not stop randomly, as per the advice above. Do I not want both a client and a pool? |
@CoreyCole You probably just want a pool. export class PostgresRepositoryService {
private pool: Pool;
constructor(private environment: EnvironmentsService) {
this.pool = new Pool({
host: this.environment.getDatabaseUrl(),
port: this.environment.getDatabasePort(),
database: 'db_name',
user: 'db_user',
password: this.environment.getDatabasePassword()
});
this.pool.on('error', (err) => {
console.error('An idle client has experienced an error', err.stack);
});
} There’s some documentation on how to use the pool at https://node-postgres.com/features/pooling. |
To handle idle pool errors see brianc/node-postgres#1324
I've been running into this issue today, and what I'm seeing doesn't seem to match what is supposed to be happening. I create my pool like this:
Then I use it like this:
When I kill the backend server, the error printed is like this:
Notice how:
If I don't add an error handler onto the client I checkout from the pool, I get an error like this when I restart the database server, and the process is terminated:
This seems counter to some of the information above that said that if a query were in progress, the error emitter would not be called. |
@dobesv similar stack happening here that I just recently noticed in our logs...
We're going to deploy a little more to our error handler to try and get more into our logs. Similar setup, we are def. listening on the pool for 'error'.
|
I've spent too much time on this, on typeorm side, but for eveyone who is struggling with this: If you read the documentation carefully(events section on https://node-postgres.com/api/client), then you'll see that connection errors are emitted on client, not thrown on client.query().Thus, all the SQL errors are thrown, or returned on the callback, but for connection errors you must attach client.on('error'), and if you don't the error will be uncaught and by default the process exits. If you look at pool.query() you see that they properly handle on('error') AND query callback errors, so if you use client returned from pool.connect you must do the same. What is weird though, as noted also by @dobesv , if you do attach on('error') then the client.query() calls will start throwing exceptions. This is inconsistent or a bug, not sure. |
Reading the docs again, the on('error') should be only important for idle connections, thus I find it is a bug, and a created a new ticket #2191 |
@joras - Hmm, doesn't the client already get the idleListener attached when its returned from pool.connect? node-postgres/packages/pg-pool/index.js Lines 46 to 59 in 4b22927
node-postgres/packages/pg-pool/index.js Line 231 in 4b22927
|
@aranair The |
@charmander - to clarify, does that mean that, ontop of the Is it correct to assume that ^^ is meant to help with the off-chance that the client gets disconnected after it is returned from |
@aranair Yes, you currently need to attach an error listener manually while the client is checked out. It’s for any connection-level errors (including getting disconnected in various ways) that happen during that time, which could be before a query, during it, after it… |
Got it, thanks for the clarification! |
…crash pg-pool requires attaching an error handler to each item checked out off the pool, which will handle connection-level errors. Without it, the application straight-up crashes whenever the connection is interrupted (e.g. database failover). See brianc/node-postgres#1324 Signed-off-by: Quentin Machu <[email protected]>
After have updated from 5.1 to 6.2.4, I see sometimes (nearly once a day) the following error which crashes our production server :
I've seen #1316
AFAIK, the error was not emitted before this patch, as of now, we have an error emitted on the pool.
I'm a bit confused here about what we should do when this error is happening ?
Does it mean that we have to reconnect the entire pool ? But how ?
Or does it means that the client emitting the error is definitively dead, so we rather drop the operation initiated by the client, and tell the user that his query is dropped. But once again how since the client.query callback seems not called with this error (perhaps somehow related to #1322) ?
The text was updated successfully, but these errors were encountered: