db.stream() has no timeout

[alxndrsn]

Currently there is no timeout on database stream connections.

This can be observed by patching gajus/slonik#347 and then creating a large number of slow, streamed connections (e.g. submission zip export), until central-backend stops responding to requests. You can check for open connections in postgres with SELECT COUNT(*) FROM pg_stat_activity;.

This may be because the version of Slonik currently in-use has various timeout settings disabled.

Slonik does not appear to implement its own stream timeout functionality.

It seems like slonik streams use pg-cursor under the hood, which may use postgres cusrors. I haven't found clear documentation of how/if postgres cursors are timed out. However, when the lockup occurs there are clear, relevant entries in both pg_stat_activity and pg_locks, but there are no rows returned by SELECT * FROM pg_cursors;. This may mean that PostgreSQL Cursors are not actually being used.

Timeouts might not be necessary if central-backend reliably destroys streams in all code paths. It's hinted this may not be the case at https://github.com/getodk/central-backend/blob/master/lib/resources/submissions.js#L282. Either way, it might be pragmatic to add timeout handling in lib/util/db.js.

Related:

• native driver ignores statement_timeout brianc/node-postgres#2103
• Closing cursor after destroying stream gajus/slonik#347
• Request soon after premature close can result in database connection leak #485
• 10 concurrent requests for OData feed results in database connection leak #482

[issa-tseng]

yeah, the current intended mechanism is that the client timeout will terminate all active queries, not just streams. of course, if the client decides to never time out that’s a problem but i think nginx will in most cases.

…

On Aug 7, 2022, at 06:09, Alex Anderson ***@***.***> wrote:  Currently there is no timeout on database stream connections. This can be observed by patching gajus/slonik#347 and then creating a large number of slow, streamed connections (e.g. submission zip export), until central-backend stops responding to requests. You can check for open connections in postgres with SELECT COUNT(*) FROM pg_stat_activity;. This may be because the version of Slonik currently in-use has various timeout settings disabled. Slonik does not appear to implement its own stream timeout functionality. It seems like slonik streams use pg-cursor under the hood, which may use postgres cusrors. I haven't found clear documentation of how/if postgres cursors are timed out. However, when the lockup occurs there are clear, relevant entries in both pg_stat_activity and pg_locks, but there are no rows returned by SELECT * FROM pg_cursors;. This may mean that PostgreSQL Cursors are not actually being used. Timeouts might not be necessary if central-backend reliably destroys streams in all code paths. It's hinted this may not be the case at https://github.com/getodk/central-backend/blob/master/lib/resources/submissions.js#L282. Either way, it might be pragmatic to add timeout handling in lib/util/db.js. Related: brianc/node-postgres#2103 gajus/slonik#347 #485 #482 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

[matthew-white]

if the client decides to never time out that’s a problem but i think nginx will in most cases.

From what I've seen, the nginx timeout doesn't have an effect on Backend: it doesn't actually terminate active queries. For example, if you send a non-GET request, and the nginx timeout kicks in, then the client will receive an error response; but Backend will continue working on the request. That means that despite the error response, the request may ultimately have its intended effect.

[alxndrsn]

From what I've seen, the nginx timeout doesn't have an effect on Backend: it doesn't actually terminate active queries.

I'm observing the same.

if you send a non-GET request, and the nginx timeout kicks in, then the client will receive an error response; but Backend will continue working on the request

@matthew-white is the behaviour different for GET requests?

[matthew-white]

is the behaviour different for GET requests?

I only remember seeing this for non-GET requests, but I would guess that it's the same for GET requests.

[matthew-white]

Closed by #604.