Reusing connections after exceptions

[torgeirsh]

When a session fails and returns Left, the connection is returned to the pool and can potentially be reused, unless it's a ClientError. However, when an exception is thrown during a session, the connection is always released and not returned to the pool for reuse. Is it generally not safe to reuse such connections, even when there isn't an explicit ClientError? Depending on how errors are modeled in an application, this can cause a high amount of seemingly unnecessary database reconnects, for example 500 times per second in a web application due to requests resulting in "not found" errors. We might have to adjust the error model to avoid triggering reconnects for such "soft" errors, but I thought I'd hear your thoughts on the matter first.

[nikita-volkov]

It looks like the release on exceptions is indeed unnecessary. What do you think @robx?

[torgeirsh]

Having thought about it some more, I suppose it's not generally safe to reuse a connection if an exception occurs. For example, consider this connection error handler in hasql. Once "False" is matched, it is known that the connection is bad and should be released instead of reused, which is signaled with ClientError. However, since LibPQ.errorMessage is an IO action, nothing prevents it from throwing an exception. Hasql-pool's exception handler has no way of knowing if the connection is bad, so it looks like always discarding the connection is the only safe thing to do. Even if the LibPQ.errorMessage actions were wrapped in catch, there could still be edge cases with asynchronous exceptions.

[nikita-volkov]

While I do agree with your thinking that it's a safe bet that might be the only one that is correct, I just want to mention that asynchronous exceptions are the only ones that should be able to be thrown during interactions with the libpq lib since it's just a wrapper over FFI.

So, do you consider the issue resolved?

[torgeirsh]

Yes, I don't think it's worth to risk possibly introducing edge case bugs related to asynchronous exceptions. I'm sure it wouldn't be fun to debug. :)