Skip to content

Fix connection management#4

Merged
bentsku merged 4 commits intomasterfrom
fix-conn-mgmt
Oct 18, 2022
Merged

Fix connection management#4
bentsku merged 4 commits intomasterfrom
fix-conn-mgmt

Conversation

@bentsku
Copy link
Copy Markdown

@bentsku bentsku commented Oct 14, 2022

This PR adds some quality of life changes to the proxy, as well as optimisations concerning selector polling (the selector would burn some CPU as it was polling on EVENT_WRITE which was always true. It would return every 1ms, now correctly wait for 1s timeout while selecting. The socket will manage if the redirected socket is writable, and if not, can always raise an error).

It adds different checks in the lifecycle on the connection, and properly terminate Postgres side with a Termination message as described by the PostgreSQL documentation (https://www.postgresql.org/docs/current/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-TERMINATE) to follow the correct message flow. Sometimes the client would disconnected without sending it, which would let the Postgres side in a ReadyForQuery state, waiting to receive something which would never come.

This came to light with this issue: prisma/prisma#11134
After reproducing locally, it seems the issue is now solved.

@bentsku bentsku requested a review from whummer October 14, 2022 16:16
@bentsku bentsku marked this pull request as ready for review October 14, 2022 17:06
Copy link
Copy Markdown
Member

@whummer whummer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kudos for digging into the performance and connection mgmt. issues with this library @bentsku ! 💪

Added a few initial comments - main question for me is the question of how to deal with the write events from the sockets. Let's proceed with testing the integration in RDS upstream, this should give us some more insights.. 👍

@bentsku bentsku requested a review from whummer October 18, 2022 14:21
Copy link
Copy Markdown
Member

@whummer whummer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @bentsku 🚀

Let's also bump the version in setup.py (perhaps this could become a minor version 0.1.0, as it contains some fairly significant changes in the logic, and also some small changes in the (internal) API). And would be great if we could start a short Changelog section in the README 👍

@bentsku
Copy link
Copy Markdown
Author

bentsku commented Oct 18, 2022

Looks great @bentsku 🚀

Let's also bump the version in setup.py (perhaps this could become a minor version 0.1.0, as it contains some fairly significant changes in the logic, and also some small changes in the (internal) API). And would be great if we could start a short Changelog section in the README 👍

Will do 👍 (I had bumped to 0.0.6 already, but 0.1.0 sounds better 👍)

@bentsku bentsku merged commit 7b8cb35 into master Oct 18, 2022
@bentsku bentsku deleted the fix-conn-mgmt branch October 18, 2022 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants