Interrupt/force close db connection when HTTP request finishes

[davidmartos96]

Is there any way to force close the database connection in a pool, even if there are queries running? The use case is to force close a connection obtained during an HTTP request in case of a timeout managed from the routes/middlewares layer in the HTTP server.
We've recently encountered with an issue on our server which made the database fill up the available connections because HTTP was timing out, but the database was still processing queries, even after the HTTP socket was closed.

We were able to achieve what we want with some tweaks in the postgres package, but I would like to hear your opinion in case you feel an alternative API would be better. Ideally without requiring extending a Pool implementation

Similar previous work:
grafana/grafana#22123

Postgres Dart fork with changes: https://github.com/davidmartos96/postgresql-dart/tree/feat/abort_conn

Custom pool, but it extends the PoolImplementation

/// A pool that aborts the connection if receives an abort signal.
/// This is useful to abort the db query if the request is aborted.
class AbortSignalAwarePool<T> extends pool_impl.PoolImplementation<T> {
  AbortSignalAwarePool(super.selector, super.settings);

  factory AbortSignalAwarePool.withEndpoints(
    List<pg.Endpoint> endpoints, {
    pg.PoolSettings? settings,
  }) =>
      AbortSignalAwarePool(pool_impl.roundRobinSelector(endpoints), settings);

  /// This method is the only one that is overridden to add the abort signal handling.
  /// and let the postgres library know about the error outside the connection callback
  /// so that it releases the connection.
  @override
  Future<R> withConnection<R>(
    Future<R> Function(pg.Connection connection) fn, {
    pg.ConnectionSettings? settings,
    locality,
  }) {
    // AbortSignal internally it's a Completer that can only be completed with an error
    // There can be an AbortSignal in the current Dart zone if we pass it through
    // in an appropriate HTTP middleware
    final AbortSignal? abortSignal = getAbortSignalInZone();

    final Future<R> Function(pg.Connection connection) effectiveFn;
    if (abortSignal == null) {
      effectiveFn = fn;
    } else {
      effectiveFn = (connection) async {
        try {
          // Internally it's a Future.any([abortSignal, fn(connection)])
          // so that the connection callbacks throws if the abort signal is triggered
          return await abortSignal.waitFuture(fn(connection));
        } catch (e) {
          if (abortSignal.wasAborted) {
            // Interrupt the pg connection if the http request has finished
            // to avoid dangling connections
            unawaited(connection.close(interruptRunning: true));
          }
          rethrow;
        }
      };
    }

    return super.withConnection(effectiveFn, settings: settings, locality: locality);
  }
}

[isoos]

We already have a way to cancel the pending statement (used for timeouts):
https://github.com/isoos/postgresql-dart/blob/master/lib/src/v3/connection.dart#L627-L641

It opens a new connection and sends the connection-specific secret token on it, which the server uses to abort the running statement (if there is any).

If we would expose it on the connection interface, would that solve this issue?

[davidmartos96]

I've tried it too, but it wasn't really closing the connection.

Thought it would be easier if I shared a reproduction case. I've created a repo with shelf+postgres and the abort utility middleware and pool extension.
It has a few tests that check for no dangling connections after the HTTP request finishes. I only managed it to make it work with the changes in the fork.
Worth mentioning that explicit closing in the AbortSignalAwarePool class is not necessary when using run, throwing an error in the withConnection callback is enough. But runTx required those extra changes in the fork.

https://github.com/davidmartos96/demo_postgres_shelf

[davidmartos96]

@isoos Would you be ok with a PR to discuss the flag that interrupts the running queries? Or are you confident the internal cancel method should work, even when using the pool abstraction? I haven't been able to make it work without editing the package.

master...davidmartos96:postgresql-dart:feat/abort_conn

[isoos]

@davidmartos96: thank you for reminding me, and please do the PR :)

Also: please bump the version + add a changelog entry for it.

I'm a bit undecided if interruptRunning is the best name for the flag, but it is possible. Otherwise does the PR resolve your original issue?

[davidmartos96]

@isoos Yes, the changes let us achieve the connection abort behavior on connections tied to HTTP requests, like HTTP timeouts, avoiding filling up the connections on unexpectedly high volume of HTTP requests and where the db is getting unresponsive.

I'll open a PR, we could discuss the name and how to test properly.

[isoos]

@davidmartos96: my memory fails me here: is there anything pending left?

[davidmartos96]

@isoos There was some alternative you mentioned in this PR comment, which wouldn't close the connection, rather cancel the current statement.

#406 (comment)

If you plan to add that maybe this issue could be updated or create a new one with the details.

I don't think there is anything "pending" besides that comment.

[isoos]

I think it should be as simple as the following code block:

  @override
  Future<void> close({bool force = false}) async {
    if (force && _pending != null) {
      await cancelPendingStatement();
    }
   ...

But tests fail with uncaught exception(s) no matter how I tweak the error handling around it. The recently added strace tracking helped a bit, but I'm afraid this needs to have a deeper rewrite before we can add that cancellation.