PG pool timeout #2248
PG pool timeout #2248
Conversation
There was a problem hiding this comment.
Benchmark
| Benchmark suite | Current: f9d0aa1 | Previous: 0d03d9f | Ratio |
|---|---|---|---|
tests/search/unit/search/test_fetch.py::test_highligh_error |
3063.137988819554 iter/sec (stddev: 0.0000033312555502632706) |
2841.0684406726436 iter/sec (stddev: 0.000004954958228416619) |
0.93 |
This comment was automatically generated by workflow using github-action-benchmark.
816a429 to
ce03e2c
Compare
8631fb9 to
f9d0aa1
Compare
| @@ -239,8 +242,7 @@ async def count(self, match: str) -> int: | |||
| class ReadOnlyPGTransaction(Transaction): | |||
| driver: PGDriver | |||
|
|
|||
| def __init__(self, pool: asyncpg.Pool, driver: PGDriver): | |||
| self.pool = pool | |||
| def __init__(self, driver: PGDriver): | |||
There was a problem hiding this comment.
Why are the read only transactions getting a driver and getting a connection every time? Shouldn't they behave as the write transactions? Get a connection, operate and finally close?
There was a problem hiding this comment.
I decided not to change the behaviour so close to the migration. But, some reasons:
- This is not running in a transaction, so there is no need to keep the connection.
- The read-only transaction may run for a while, e.g: for the entire duration of a read request. This would block the connection for this entire duration of the request, so in practice we would need one connection to PG per concurrent read request.
- Checking out and returning a connection to the pool is pretty fast (it doesn't involve networking unless the connection is inside a transaction when it is returned to the pool, in which case you send a rollback). So, checkout+release is not to costly.
I think this approach works better in the face of long-running read transactions with little DB activity, and we have a clear case of that in search() (waiting for the index to reply). So by keeping to do this, we would use up less of our open connections (which are pretty limited).
Description
Adds a timeout for all PG pool
acquire()calls, including implicit ones (e.g:self.pool.execute()implicitly checks out a connection without timeout).This makes any error regarding pool connection exhaustion much more visible, rather than taking too long or getting stuck forever.