Introduce on-demand connection acquisition mode

[arybczak]

No description provided.

[crtschin]

Nice work! I didn't spot anything particularly wrong, just have a few questions. I do think this warrants a bit more documentation to the intended usage or intuition behind these connection modes.

[crtschin]

Oh, I was reviewing an older commit. That sucks, the line numbers in my review don't match.

[crtschin]

Previously reviewed an old commit, moved some comments around and also took a look at newer changes.

[crtschin]

I don't think we preserve this currently.

• getConnectionAcquisitionMode uses readMVar which is interruptible.
• runQuery contains takeMVar which is also interruptible.

Both of these imply the uninterruptibleMask will hang if an exception lands while waiting on those blocking functions, (which implies deadlocks if the action we're waiting on was the one that caused the exception). Though I think this is fine as long as connections are never used concurrently by multiple threads.

[arybczak]

We do, that's exactly what uninterruptibleMask does (which you wrote later in the comment, so I'm confused about the first sentence 🤔 ).

Good point about a potential dead lock, I narrowed down the scope of uninterruptibleMask, it should be fine now.

[Raveline]

Looks generally good to me, though the heavy use of uninterruptibleMask is somewhat scary. As Curtis pointed to in his own review, we could end up with some never-ending, unkillable threads if a takeMVar hangs (though I don't see a particular reason why it would).

The rationale you offer is that it guarantees "we don't end up in unexpected transaction state", but I would argue this should not be a responsability of the library, but rather of client code. I do believe a simple mask_ (as postgresql-simple use for transactions) would be more than enough, but since there was already usage of uninterruptableMask_ before this PR, I suppose this can be considered as already having been battle-tested to some extent.