Patch: Connection stuck after a while

[wackfx]

Hey @porsager

I noticed that, after a while, I had random "WRITE_TIMEOUT" // "CONNECT_TIMEOUT" happening and my system became unresposive.

It appears that the socket, once closed, doesn't correctly reconnect. By recreating the socket, it looks like it solved the issue. Today, this behavior is done only on "secured" but I believe we could have it for all connections.

I believe it's ok to do so as 1) it shouldn't happen often 2) socket is closed and will eventually be garbage collected 3) it's already done for secured connections

Reproducing & checks

Context: I'm using Bun
I started with Postgres down - and started postgres after the first attempt to try to reproduce #641

⚠️ It looks like this behavior doesn't happen on Node - when using node with or without my patch, the socket correctly reconnects. I believe it may be solved by some Node shenanigan and impacting Bun (or even Worker?) only.

import postgres from 'postgres'

const sql = postgres(`${process.env['DATABASE_URL']}`, {
    max: 1, # trigger the issue sooner
    max_lifetime: 1, # trigger the issue sooner by closing the connection really quickly
})

setInterval(async () => {
    console.log((await sql`SELECT * FROM pg_am`)[0])
}, 3000)

Before

After the first query - connection is closed (because of max_lifetime: 1) and doesn't reconnect

After

It reconnects well.

[dimonnwc3]

@wackfx thanks for contribution. I've experienced the same, connection in bun times out and then postgres is not available anymore, with the following error: write CONNECT_TIMEOUT postgres:5432

before it gets merged, a workaround is to use a "secured" connection?

[wackfx]

That's correct. Usng a secured connection (so with SSL connection, or give a custom encrypted socket at connection time) would produce the same behaviour as this PR. -------- Message d'origine -------- Le 26 nov. 2023, 09:42, Dmitrii a écrit :

…

***@***.***(https://github.com/wackfx) thanks for contribution. I've experienced the same, than connection in bun times out and then postgres is not available anymore, with the following error: write CONNECT_TIMEOUT postgres:5432 before it gets merged, a workaround is to use a "secured" connection? — Reply to this email directly, [view it on GitHub](#738 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/BAHITRUCZN4KE3JVBCERAW3YGL6GZAVCNFSM6AAAAAA7YOHSZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRWG4ZDIMZZGI). You are receiving this because you were mentioned.Message ID: ***@***.***>

[porsager]

Very nice debugging, and thanks for the proposed fix @wackfx, but I actually think this should be fixed on Buns end. Making workarounds for Bun bugs in all libraries is just a useless work multiplier. If we were using native Bun apis, it would be a completely different matter, but this is Bun wanting to mimick Node.js APIs, so I'm certain they'd also want to have a bug like this fixed. - Did you make an issue with Bun, and a minimal reproducible case using only net (tcp)?

[porsager]

Even so, you are also right, there shouldn't be any actual performance concerns or issues recreating the socket for non ssl connections too, so let's take your simplification 😉

[wackfx]

hey @porsager - you're very right about the fact that Bun might wanna take a look into this for a better Node compat, tbh I opened this before trying to reproduce with Node.JS.

[dimonnwc3]

@porsager any plans on releasing this to npm?

[Th1nkK1D]

Amazing findings! I ran into the same issue. While waiting for a release, I use the following command to temporarily patch connection.js from this PR.

curl https://raw.githubusercontent.com/porsager/postgres/6f20a4820c683b33e7670b606d8daf5670f4b973/src/connection.js > node_modules/postgres/src/connection.js

PS. I added it in my docker file after bun install, but you and also run it manually.

[mattbishop]

Probably can remove the two individual socket.removeListener() calls a few lines before this change too.

[WildEgo]

Can we publish this..? It's an high impact bug fix

[hex2f]

any update on when this will make it into a release? its causing me a lot of headaches right now 😅

[emilienbidet]

It cause me a lot of troubles too. Latest release is from November, may we can make a new one?

[porsager]

A reminder at the right time ;) Just released v3.4.4

[edwardchew97]

A reminder at the right time ;) Just released v3.4.4

Deployed the latest version yesterday, 12 hours passed and so far it seems to resolve the issue in our case! 🙌

[porsager]

Fantastic! Mind sharing what you're running on? Bun?

[porsager]

@edwardchew97 are you using bun?

[kam-st]

Hello, so pulling directly from github to resolve cloudflare issue. But the issue of queries getting stuck persists. The way I reproduce stuck queries is through repetitive refresh of page. The queries though authjs gets stuck and whole server gets jammed.

Stack is NextJS | Drizzle | AuthJS (Drizzle running through postgresjs driver)

The reason I know this issue is with postgresjs is because if I use neon drivers. Then the issue does not persists and everything works flawlessly.

PS: I am using nodeJS

[gregory-krieger]

This is the solution I implemented, just replace this :

import "dotenv/config";

import { drizzle } from "drizzle-orm/postgres-js";
import postgres from "postgres";
import * as schema from "./schema";

const connectionString = process.env.DATABASE_URL;

// Disable prefetch as it is not supported for "Transaction" pool mode
export const client = postgres(connectionString!, {
  prepare: false,
  ssl: false,
});
export const db = drizzle(client, { schema });

with this :

import "dotenv/config";
import { PostgresJsDatabase, drizzle } from "drizzle-orm/postgres-js";
import postgres from "postgres";
import * as schema from "./schema";

declare global {
  var database: PostgresJsDatabase<typeof schema> | undefined;
}

const connectionString = process.env.DATABASE_URL;

if (process.env.NODE_ENV === "production") {
  database = drizzle(
    postgres(connectionString!, {
      prepare: false,
    }),
    { schema },
  );
} else {
  if (!global.database)
    global.database = drizzle(
      postgres(connectionString!, {
        prepare: false,
      }),
      { schema },
    );
  database = global.database;
}
export const db = database;

I'm using Drizzle + Supabase + Next.js