porsager / postgres

Postgres.js - The Fastest full featured PostgreSQL client for Node.js, Deno, Bun and CloudFlare
The Unlicense
7.03k stars 255 forks source link

BUG: All further queries hang if transaction scoped sql parameter is not used in some queries [with reproduction code] #893

Open chanon opened 2 weeks ago

chanon commented 2 weeks ago

So, I was having an issue where my current in development web service endpoint would become completely unresponsive. Tracing showed that when it happens, what happens is it just goes silent after beginning a transaction. No debug outputs come out inside the transaction callback.

After this happens all further requests to postgres.js sql connection just hang. No error/exceptions or timeouts. The promises just quietly never resolve, requiring a complete application restart.

Something else that I noticed was that it would tend to happen when my app makes a lot of concurrent requests.

As the endpoint is extremely simple, I started writing a minimal reproduction, which didn't reproduce the problem at first. Until I noticed something in my code. I wasn't using the sql parameter that was being given to the callback in the sql.begin call.

See complete reproduction below which doesn't even require touching any database tables.

It doesn't happen at low concurrency. At higher concurrency (in my tests starting at about 10), it always happens. The program just hangs and even correctly written transactions then become stalled too.

import postgres from 'postgres'

const sql = postgres({ /* options */ }) // will use psql environment variables

const wrongTransactionUse = async (input) => {
    let result = 'wrong: ' + 
        (await sql.begin(async _ => await sql`SELECT ${input} as val`))[0].val
    console.log(result)
    return result
}

const correctTransactionUse = async (input) => {
    let result = 'correct: ' + 
        (await sql.begin(async sql => await sql`SELECT ${input} as val`))[0].val
    console.log(result)
    return result
}

const testInterleaved = async () => {
    const promises = []
    const runCount = 100  // try lowering this to 5 and it will work
    for (var i = 0; i < runCount; i++) {
        promises.push(wrongTransactionUse(i))
        promises.push(correctTransactionUse(i))
    }
    await Promise.all(promises)
    sql.end()
}
testInterleaved()

// const doTimes = async (func, times) => {
//  const promises = []
//  for (var i = 0; i < times; i++) {
//      promises.push(func(i))
//  }
//  console.log(await Promise.all(promises))
// }
// 
// const testSets = async () => {
//  for (var i = 0; i < 100; i++) {
//      await doTimes(correctTransactionUse, i)
//      await doTimes(wrongTransactionUse, i)
//  }
//  sql.end()
// }
// testSets() // maybe also take a look at this to see how much concurrency it starts to fail

Output when runCount is 100

correct: 0
correct: 3
correct: 2
correct: 1
correct: 4
correct: 5
correct: 6
correct: 7
correct: 8
| 
(it hangs, cursor left blinking)

Output when runCount is 5

correct: 1
correct: 3
correct: 0
correct: 2
correct: 4
wrong: 0
wrong: 2
wrong: 1
wrong: 4
wrong: 3
(program ends correctly)