Skip to content

Commit da25ebf

Browse files
committed
fix(query): ensure that open connection are killed after timeout
Without statement_timeout set, the query_timeout wont always kill the underlying database query connection leading to possible connections exhaustions
1 parent 76cd87a commit da25ebf

File tree

4 files changed

+52
-2
lines changed

4 files changed

+52
-2
lines changed

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@
3030
"test": "run-s db:clean db:run test:run db:clean",
3131
"db:clean": "cd test/db && docker compose down",
3232
"db:run": "cd test/db && docker compose up --detach --wait",
33-
"test:run": "PG_META_MAX_RESULT_SIZE_MB=20 vitest run --coverage",
34-
"test:update": "run-s db:clean db:run && PG_META_MAX_RESULT_SIZE_MB=20 vitest run --update && run-s db:clean"
33+
"test:run": "PG_META_MAX_RESULT_SIZE_MB=20 PG_QUERY_TIMEOUT_SECS=3 PG_CONN_TIMEOUT_SECS=30 vitest run --coverage",
34+
"test:update": "run-s db:clean db:run && PG_QUERY_TIMEOUT_SECS=3 PG_CONN_TIMEOUT_SECS=30 vitest run --update && run-s db:clean"
3535
},
3636
"engines": {
3737
"node": ">=20",

src/server/constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ export const PG_META_MAX_RESULT_SIZE = process.env.PG_META_MAX_RESULT_SIZE_MB
5959
export const DEFAULT_POOL_CONFIG: PoolConfig = {
6060
max: 1,
6161
connectionTimeoutMillis: PG_CONN_TIMEOUT_SECS * 1000,
62+
// node-postgrest need a statement_timeout to kill the connection when timeout is reached
63+
// otherwise the query will keep running on the database even if query timeout was reached
64+
statement_timeout: PG_QUERY_TIMEOUT_SECS * 1000 - 1,
6265
query_timeout: PG_QUERY_TIMEOUT_SECS * 1000,
6366
ssl: PG_META_DB_SSL_ROOT_CERT ? { ca: PG_META_DB_SSL_ROOT_CERT } : undefined,
6467
application_name: `postgres-meta ${pkg.version}`,

test/index.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,4 @@ import './server/ssl'
2323
import './server/table-privileges'
2424
import './server/typegen'
2525
import './server/result-size-limit'
26+
import './server/query-timeout'

test/server/query-timeout.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { expect, test, beforeAll, afterAll, describe } from 'vitest'
2+
import { app } from './utils'
3+
import { pgMeta } from '../lib/utils'
4+
5+
describe('test query timeout', () => {
6+
beforeAll(async () => {
7+
// Create a table for testing
8+
await pgMeta.query(`
9+
CREATE TABLE timeout_test (
10+
id SERIAL PRIMARY KEY,
11+
data TEXT
12+
);
13+
`)
14+
})
15+
16+
afterAll(async () => {
17+
// Clean up the test table
18+
await pgMeta.query('DROP TABLE timeout_test;')
19+
})
20+
21+
test('query timeout after 5s and connection cleanup', async () => {
22+
const query = `SELECT pg_sleep(10);`
23+
// Execute a query that will sleep for 10 seconds
24+
const res = await app.inject({
25+
method: 'POST',
26+
path: '/query',
27+
payload: {
28+
query,
29+
},
30+
})
31+
32+
// Check that we get the proper timeout error response
33+
expect(res.statusCode).toBe(408) // Request Timeout
34+
expect(res.json()).toMatchObject({
35+
error: expect.stringContaining('Query read timeout'),
36+
})
37+
38+
// Verify that the connection has been cleaned up by checking active connections
39+
const connectionsRes = await pgMeta.query(`
40+
SELECT * FROM pg_stat_activity where application_name = 'postgres-meta 0.0.0-automated' and query ILIKE '%${query}%';
41+
`)
42+
43+
// Should have no active connections except for our current query
44+
expect(connectionsRes.data).toHaveLength(0)
45+
})
46+
})

0 commit comments

Comments
 (0)