Skip to content

Commit

Permalink
remove clients from the pool following any error
Browse files Browse the repository at this point in the history
we're not confident in our ability to accurately determine what a "bad"
error (i.e. one that has rendered the connection broken). to be safe,
we'll instruct the pool to remove any client that failed to execute
a query

though this will result in more pool churn, we're using pgbouncer so
those connections are cheap, and this provides maximum safety overall
  • Loading branch information
clarkdave committed Nov 21, 2019
1 parent e1926af commit 399b818
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 7 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@loyaltylion/typeorm",
"private": true,
"version": "0.2.20-1",
"version": "0.2.20-2",
"description": "loyaltylion fork",
"license": "MIT",
"readmeFilename": "README.md",
Expand Down
30 changes: 24 additions & 6 deletions src/driver/postgres/PostgresQueryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,30 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner
this.isReleased = true;

if (this.releaseCallback) {
// severe errors may indicate an unrecoverable client, e.g. a client
// whose connection no longer exists (due to network issues, etc).
// the safest thing to do is instruct the pool to remove it, or it
// will stick around in the pool indefinitely, breaking every
// subsequent query that runs on it
const removeFromPool = Boolean(error && error.severity === "FATAL");
// we have some options here:
//
// 1. remove the client from the pool no matter what error we get.
// this will be more inefficient overall, because we'll be
// removing "good" clients, i.e. those which encountered a unique
// violation error (the connection is still good)
//
// 2. try to determine if it's a "good" error (unique violation etc)
// and if it is, don't remove it from the pool
//
// 3. try to determine if it's a "bad" error ("connection
// terminated" etc) and, if it is, remove the client from the
// pool
//
// the error we received is *not* the underlying error, but a
// `QueryFailedError` which is constructed by copying all the errors
// props. so all we have to go on is the error message and presence
// of props
//
// for this reason, we've decided for now to just be safe and drop a
// client which has any error. this will mean more dropped clients,
// but we don't expect that to be an issue because they're fairly
// cheap pgbouncer connections anyway
const removeFromPool = Boolean(error);

this.releaseCallback(removeFromPool);
}
Expand Down

0 comments on commit 399b818

Please sign in to comment.