pwtail / vinyl_v1

2 stars 1 forks source link

Make the pool size configurable #7

Open dvarrazzo opened 2 years ago

dvarrazzo commented 2 years ago

https://github.com/vinylproject/vinyl/blob/4e0a7e1b4c30bb4a6887b4db59992961920f1ee7/vinyl/postgresql_backend/sync.py#L12-L15

What is the point of a connection pool with exactly one connection? You are effectively serializing all the requests, and might even create a deadlock if two connections are requested at the same time.

Maybe min_size=1, max_size=4 is a better default, however I would leave these entries configurable.

pwtail commented 2 years ago

Hi @dvarrazzo The thing is this is a sync backend, and DatabaseWrapper instances are created per-thread. So it means that I have 1 pool per thread. The pool is actually not needed at all because 1 thread only needs 1 connection at a time.

But I use the pool here as a logical entity. Because even if you need only one connection, that connection also has a lifetime after which it should be recreated, the health checks should be made from time to time, etc - the very same things you do to connections in the pool. So I need the pool just to reuse the logic of working with a single connection.

Now, about the size: I think, size=1 corresponds to the current logic of django's own postgresql backend. But you can probably have 2 connections when you know 1 connection is about to die so that to return the new connection immediately. But that is kind of optimisation.