Skip to content

Commit

Permalink
fix(postgres): invalidate connection after client-side timeout (#15283)
Browse files Browse the repository at this point in the history
* fix(postgres): invalidate connection after client-side timeout

Merge ff43e8d from main:

The `query_timeout` feature of the `pg` package helps handle stuck TCP
connections more quickly and gracefully by implementing a client-side
timeout:

brianc/node-postgres#1713

Sequelize started passing this dialect-specific option through to `pg` here:

#13258

I believe we also want to invalidate the connection when a client-side
timeout occurs. We shouldn't try to reuse the stuck connection
because...it's stuck.

This PR updates the error handling code so that the connection is
invalidated if the error matches the one thrown from here:

https://github.com/brianc/node-postgres/blob/5538df6b446f4b4f921947b460fe38acb897e579/packages/pg/lib/client.js#L529

* fix syntax error

* fix another syntax error

* fix import
  • Loading branch information
harrykao committed Nov 15, 2022
1 parent 67e69cd commit a205765
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/dialects/postgres/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ class Query extends AbstractQuery {
|| /Unable to set non-blocking to true/i.test(error)
|| /SSL SYSCALL error: EOF detected/i.test(error)
|| /Local: Authentication failure/i.test(error)
// https://github.com/sequelize/sequelize/pull/15144
|| error.message === 'Query read timeout'
) {
connection._invalid = true;
}
Expand Down
47 changes: 46 additions & 1 deletion test/integration/dialects/postgres/query.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ const chai = require('chai'),
expect = chai.expect,
Support = require('../../support'),
dialect = Support.getTestDialect(),
DataTypes = require('sequelize/lib/data-types');
DataTypes = require('sequelize/lib/data-types'),
DatabaseError = require('sequelize/lib/errors/database-error');

if (dialect.match(/^postgres/)) {
describe('[POSTGRES] Query', () => {
Expand Down Expand Up @@ -188,5 +189,49 @@ if (dialect.match(/^postgres/)) {
order: [['order_0', 'DESC']]
});
});

describe('Connection Invalidation', () => {
if (process.env.DIALECT === 'postgres-native') {
// native driver doesn't support statement_timeout or query_timeout
return;
}

async function setUp(clientQueryTimeoutMs) {
const sequelize = Support.createSequelizeInstance({
dialectOptions: {
statement_timeout: 500, // ms
query_timeout: clientQueryTimeoutMs
},
pool: {
max: 1, // having only one helps us know whether the connection was invalidated
idle: 60000
}
});

return { sequelize, originalPid: await getConnectionPid(sequelize) };
}

async function getConnectionPid(sequelize) {
const connection = await sequelize.connectionManager.getConnection();
const pid = connection.processID;
sequelize.connectionManager.releaseConnection(connection);

return pid;
}

it('reuses connection after statement timeout', async () => {
// client timeout > statement timeout means that the query should fail with a statement timeout
const { sequelize, originalPid } = await setUp(10000);
await expect(sequelize.query('select pg_sleep(1)')).to.eventually.be.rejectedWith(DatabaseError, 'canceling statement due to statement timeout');
expect(await getConnectionPid(sequelize)).to.equal(originalPid);
});

it('invalidates connection after client-side query timeout', async () => {
// client timeout < statement timeout means that the query should fail with a read timeout
const { sequelize, originalPid } = await setUp(250);
await expect(sequelize.query('select pg_sleep(1)')).to.eventually.be.rejectedWith(DatabaseError, 'Query read timeout');
expect(await getConnectionPid(sequelize)).to.not.equal(originalPid);
});
});
});
}

0 comments on commit a205765

Please sign in to comment.