jackc / pgx

PostgreSQL driver and toolkit for Go
MIT License
10.91k stars 849 forks source link

Is pgx.pool.Acquire() multithread-ready? #1055

Open nuliknol opened 3 years ago

nuliknol commented 3 years ago

During debugging I have found that one of my go-routines got sutck on pgx.pool.Acquire() function. The thing is, I have multiple go-routines using single pool. So, the question is, is pgx.pool safe against deadlocks ???

I am using the pool with CopyFrom and Exec, so, Acquire should release any resouces if those were allocated and free the connection. But I am seening that on request number 26 I am getting blocked inside the pool.Acquire() function

nuliknol commented 3 years ago

here is my config to connect, just in case:

func DbsConnectPool() (*pgxpool.Pool,error) {

    host,port,err:=net.SplitHostPort(os.Getenv("DB_HOST"))
    if (err!=nil) {
        host = "localhost"
        port="5432"
    }   
    conn_str := "user='"+ os.Getenv("USERNAME") +
        "' dbname='" + os.Getenv("DATABASE") +
        "' password='" + os.Getenv("PASSWORD") +
        "' host='" + host +
        "' port='" + port + "'";

    pgx.Connect(context.Background(),conn_str)
    pool_cfg,err := pgxpool.ParseConfig(conn_str)
    if err != nil {
        panic(fmt.Sprintf("Err parsing config: %v\n",err))
    }   
    pool_cfg.MaxConns = MAX_CONN
    pool_cfg.MinConns = MIN_CONN
    pool_cfg.AfterConnect = after_connect_func
    pool_cfg.LazyConnect = false
    pool,err := pgxpool.ConnectConfig(context.Background(),pool_cfg)
    return pool,err
}
nuliknol commented 3 years ago

Replaced pgx.pool with individual connections (not because of this bug, but because I saw there is some logic in pool management that I don't need, so I decided to go for individual connection per business-logic function to shave on clock cycles) , and dead locks disappear. So it is very probable that there is some race condition in pgx.pool that can lock it.

echlebek commented 3 years ago

https://github.com/jackc/puddle/issues/8 describes a deadlock that is possible in puddle, which I gather is used for pooling in pgx. This has been released and incorporated into pgx, but pgx has not been released yet.

@jackc could you perhaps cut a release so that this issue can be re-tested? Perhaps this deadlock is the one the issue submitter experienced. We have run into a similar issue and are keen to see if this resolves is. Many thanks!

bfontaine commented 2 years ago

One year later, is this issue still relevant?

alexelisenko commented 2 years ago

Was https://github.com/jackc/puddle/issues/8 fix merged into pgxpool? I am also experiencing an occasional deadlock, using only the Query and Exec methods.

jackc commented 2 years ago

Yes, there have been several fixes since then. In particular v5.1.0 and higher have a significantly improved pool implementation.

alexelisenko commented 2 years ago

@jackc great, I was using v5.0.4, will try v5.1.1 now, thanks!

danludwig commented 1 year ago

I thought I ran into something like this, but it was deadlocking because I had no defer poolconn.Release() after poolconn.Acquire(c)

mspi92 commented 1 year ago

I had a deadlock issue but i think figured it out. Some hints: Problem a)

row := pool.QueryRow(context.Background(), "...")
if conditional {
    // pool connection is first released after row.Scan is executed --> leak
    return 
}
row.Scan(...)

Problem b)

// pool with max_conn == 1
rows, err := pool.Query(context.Background(), "...")
...
defer rows.Close()
for rows.Next() {
    ...
    // connections are exceeded now --> deadlock bcs rows.Close() will never be called
    rows2, err := pool.Query(context.Background(), "...")
    ...
}