Skip to content

fix(query): ensure that open connection are killed after timeout #946

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
"test": "run-s db:clean db:run test:run db:clean",
"db:clean": "cd test/db && docker compose down",
"db:run": "cd test/db && docker compose up --detach --wait",
"test:run": "PG_META_MAX_RESULT_SIZE_MB=20 vitest run --coverage",
"test:update": "run-s db:clean db:run && PG_META_MAX_RESULT_SIZE_MB=20 vitest run --update && run-s db:clean"
"test:run": "PG_META_MAX_RESULT_SIZE_MB=20 PG_QUERY_TIMEOUT_SECS=3 PG_CONN_TIMEOUT_SECS=30 vitest run --coverage",
"test:update": "run-s db:clean db:run && PG_META_MAX_RESULT_SIZE_MB=20 PG_QUERY_TIMEOUT_SECS=3 PG_CONN_TIMEOUT_SECS=30 vitest run --update && run-s db:clean"
},
"engines": {
"node": ">=20",
Expand Down
3 changes: 3 additions & 0 deletions src/server/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ export const PG_META_MAX_RESULT_SIZE = process.env.PG_META_MAX_RESULT_SIZE_MB
export const DEFAULT_POOL_CONFIG: PoolConfig = {
max: 1,
connectionTimeoutMillis: PG_CONN_TIMEOUT_SECS * 1000,
// node-postgrest need a statement_timeout to kill the connection when timeout is reached
// otherwise the query will keep running on the database even if query timeout was reached
statement_timeout: (PG_QUERY_TIMEOUT_SECS + 1) * 1000,
query_timeout: PG_QUERY_TIMEOUT_SECS * 1000,
ssl: PG_META_DB_SSL_ROOT_CERT ? { ca: PG_META_DB_SSL_ROOT_CERT } : undefined,
application_name: `postgres-meta ${pkg.version}`,
Expand Down
1 change: 1 addition & 0 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ import './server/ssl'
import './server/table-privileges'
import './server/typegen'
import './server/result-size-limit'
import './server/query-timeout'
33 changes: 33 additions & 0 deletions test/server/query-timeout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { expect, test, describe } from 'vitest'
import { app } from './utils'
import { pgMeta } from '../lib/utils'

describe('test query timeout', () => {
test('query timeout after 3s and connection cleanup', async () => {
const query = `SELECT pg_sleep(10);`
// Execute a query that will sleep for 10 seconds
const res = await app.inject({
method: 'POST',
path: '/query',
payload: {
query,
},
})

// Check that we get the proper timeout error response
expect(res.statusCode).toBe(408) // Request Timeout
expect(res.json()).toMatchObject({
error: expect.stringContaining('Query read timeout'),
})
// wait one second for the statement timeout to take effect
await new Promise((resolve) => setTimeout(resolve, 1000))

// Verify that the connection has been cleaned up by checking active connections
const connectionsRes = await pgMeta.query(`
SELECT * FROM pg_stat_activity where application_name = 'postgres-meta 0.0.0-automated' and query ILIKE '%${query}%';
`)

// Should have no active connections except for our current query
expect(connectionsRes.data).toHaveLength(0)
}, 5000)
})