Skip to content

feat: improved named prepared statements support #694

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

Open
wants to merge 72 commits into
base: main
Choose a base branch
from

Conversation

v0idpwn
Copy link
Member

@v0idpwn v0idpwn commented Jul 7, 2025

Adds support for Prepared Statements through the Extended Query Protocol.

Concept

This PR implements the following architecture:

  • ClientHandlers keep track of the prepared statements sent on them
  • ClientHandlers update the prepared statement packets changing the statement names to "internal names" (this is necessary because clients may reuse the names in different connections)
  • Prepared statement packets aren't sent directly to the db socket, but instead are sent to the DbHandler. Whenever the ClientHandler sends a bind packet to the DbHandler, it should also send the respective parse (which it has stored)
  • DbHandler decides whether it needs to run "parse" or just "bind"
  • DbHandler must intercept "parse" responses when the client didn't request them/it was made due to necessity
  • DbHandler injects "parse" responses when the client requested a parse but it's not necessary

Parsing/decoding

  • To be able to handle the prepared statement packets, ClientHandlers need to peek at every client packet (at least on transaction mode)
  • To be able to intercept parse responses, DbHandler needs to peek at every server packet.
  • For both ClientHandler and DbHandler, we can avoid decoding the whole payload/decode only what's necessary (generally, we only need tagging, except for the prepared statement payloads from which we need more data).
  • The decoding means more CPU usage, and it could also mean that we will need to optimize the decoding routines a bit further.

Limits

  • ClientHandlers and DbHandler have separate limits
  • If trespassing a ClientHandler limit, an error is returned and the connection is closed
  • If trespassing the DbHandler limit, a clean-up is triggered and a random set of prepared statements is dropped. This is transparent to the client. The only consequence for them is that some of their queries may need to be re-prepared.
  • ClientHandler has a memory limit. Every client connection is limited to (currently) 1MB of parse statements.

Potential next steps

  • Make it work with read replicas too
  • Smarter prepared statement eviction on DbHandler. E.g.: using a LRU strategy to evict the prepared statements that weren't used recently.
  • Configurable limits
  • Performance improvements/reducing CPU usage

Additional features

  • Added a simple feature flag system per tenant
  • Added some inspect/packet debugging utilities

v0idpwn added 2 commits July 8, 2025 00:00
It's suboptimal to send a lot of small packets, instead we should try to chunk when possible to ensure better network utilization
@@ -256,7 +270,10 @@
def handle_event(:info, {proto, _, bin}, _, %{replica_type: :read} = data)
when proto in @proto do
Logger.debug("DbHandler: Got read replica message #{inspect(bin)}")
pkts = Server.decode(bin)

# TODO: use streaming for read replica too

Check warning

Code scanning / Credo

Found a TODO tag in a comment: # TODO: use streaming for read replica too Warning

Found a TODO tag in a comment: # TODO: use streaming for read replica too
@v0idpwn v0idpwn force-pushed the feat/prepared-statements-support branch from 7f45f6e to 4cd2835 Compare August 14, 2025 13:01
fix dialyzer, remove bad docs

wip
@v0idpwn v0idpwn force-pushed the feat/prepared-statements-support branch from 4cd2835 to 2badfa1 Compare August 14, 2025 13:03
@v0idpwn v0idpwn force-pushed the feat/prepared-statements-support branch from 1e704e9 to e5d13c0 Compare August 14, 2025 15:27
@v0idpwn v0idpwn force-pushed the feat/prepared-statements-support branch from 30ffe54 to 50d4042 Compare August 14, 2025 15:57
@v0idpwn
Copy link
Member Author

v0idpwn commented Aug 14, 2025

Pushed some architecture changes. Instead of using Supavisor.Protocol.split_pkts/1, which requires buffering all packets, implemented Supavisor.Protocol.MessageStreamer, which allows us to buffer only the packets we need to handle. This helps a lot avoiding performance issues for large messages (such as COPY or big data rows).

Also did some refactoring for better splitting of responsibilities, and a feature flag for prepared statements.

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