Hacker News new | ask | show | jobs
by frollogaston 10 days ago
That's an easy way to accidentally leave a xact open way too long. You might have enough connections in to support this normally, but when things go slightly wrong, they go very wrong.
2 comments

+1 to this - I've griped pretty often that FastAPI's documentation implicitly recommends this (https://fastapi.tiangolo.com/tutorial/sql-databases/#create-...) by suggesting using dependency injection to manage database connections, only to start seeing connection pool exhausted errors as soon as the number of concurrent requests exceeds the number of allowed connections.
FastAPI pattern works very well with Pgbouncer, when it is in transaction pool mode.

Your Python application maintains a connection to Pgbouncer during the lifecycle of the request, but the physical Postgres connection is allocated only during the DB transaction. You will need open/close transactions in your code though.

This is why I said PgBouncer is a sign of something being wrong. Devs aren't managing connections right, they try to paper over it with PgBouncer, it's not really easier cause they now need to be conscious of xacts instead, and now there's an extra moving part in the DB that most of the team doesn't really understand. PgBouncer has its other uses, but I really don't like this one.

I also get it, xact should be 1:1 with connection in a lot of these backend applications. Sometimes I have a few little helpers for that, like pool.sql() will take conn, open xact, execute, close xact, return conn. If the DB driver doesn't already have that.

Oh wow. Dep injection for DB connections is nasty.
I might be outing myself as a noob here, but... what is the (better) alternative?
You inject the pool itself.
Sorry, I am being dense... how does that solve the problem?

I still have to get a connection from the pool, I just do it inside the function body now, right?

So this

    @app.get("/users")
    def get_users(conn = Depends[get_db_conn]):
        users = conn.execute("SELECT * FROM users")
        return users
would become that instead:

    @app.get("/users")
    def get_users(pool = Depends[get_db_pool]):
        with pool.get_conn() as conn:
            users = conn.execute("SELECT * FROM users")
        return users
But I still need enough connections in the pool to handle all concurrent requests, no?
The idea is you only take a connection from the pool when you need to touch the DB, then you give it back immediately. It's very possible that's only a small fraction of the time spent in some handlers. If you inject the connection, you always hold it through the entire request.
Every RDBMS out there has an option to configure a DB-enforced transaction timeout. But that configuration is tied to a connection, not to a transaction. So using that configuration means giving up connection pooling. Which a lot of people don't want to do because it impacts latency and DB resource usage.
Every xact within a given connection will use the same connection-wide config, but the timeout is counting how long a single transaction takes, right? I don't see why you'd need to give up pooling for this unless you need different settings per xact.
Pooling means you're sending multiple queries (from different HTTP requests) over the same DB connection. A well-designed DB wire protocol will allow for pipelining those queries:

[send Query 1] -> [send Query 2] -> [send Query 3] -> [receive Result 1] -> [receive Result 2] -> [receive Result 3]

But in Postgres and MySQL, pipelined queries are not executed in parallel. They're just queued up for a single thread (per connection) to execute sequentially. Thus, if Query 1 is a transaction that takes too long, then it ends up blocking the execution of Query 2 and Query 3.