-
Notifications
You must be signed in to change notification settings - Fork 285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Patch: Connection stuck after a while #738
Conversation
@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: before it gets merged, a workaround is to use a "secured" connection? |
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: ***@***.***>
|
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 |
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 😉 |
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. |
@porsager any plans on releasing this to npm? |
Amazing findings! I ran into the same issue. While waiting for a release, I use the following command to temporarily patch 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 |
Probably can remove the two individual |
Can we publish this..? It's an high impact bug fix |
any update on when this will make it into a release? its causing me a lot of headaches right now 😅 |
It cause me a lot of troubles too. Latest release is from November, may we can make a new one? |
A reminder at the right time ;) Just released v3.4.4 |
Fantastic! Mind sharing what you're running on? Bun? |
@edwardchew97 are you using bun? |
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 |
This is the solution I implemented, just replace this :
with this :
I'm using Drizzle + Supabase + Next.js |
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
Before
After the first query - connection is closed (because of
max_lifetime: 1
) and doesn't reconnectAfter
It reconnects well.