Skip to content

Better handling extended protocol messages in the event of busy pool #155

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

Merged

Conversation

drdrsh
Copy link
Collaborator

@drdrsh drdrsh commented Sep 1, 2022

Let's imagine a Pgcat instance with single pool containing a single instance. The pool is fully occupied and all available connections are fully booked.

A client comes in, it connects and is now sitting idle in the external loop https://github.com/levkk/pgcat/blob/main/src/client.rs#L573

The client attempts to perform an extended protocol query. So first thing it does is send a P message. We try to grab a connection from the pool here but we fail. This failure sends us to this branch that ends with continue. The P message never got buffered because we buffer message in the inner loop here. Subsequent extended protocol messages meet similar fate and now the client sends a sync S message, This one manages to break through, so we are able to grab a connection and we enter the inner loop (The transaction loop) but we arrived here without any of the previous messages being buffered so we end up with errors like ERROR: unnamed prepared statement does not exist or ERROR: portal "" does not exist. This does not affect clients that are already in the inner loop and have a server connection allocated.

This change allows us to buffer the extended protocol message early without even checking out a connection from the pool. The actual checkout will be performed when the S message arrives, at this point, we are guaranteed to have all messages buffered and have a client on the other end expecting a response. We can respond with SystemError: Failed to get a connection from the Pool or execute the query.

@levkk
Copy link
Contributor

levkk commented Sep 1, 2022

Cool!

@levkk levkk merged commit 7f20dc3 into postgresml:main Sep 1, 2022
jmeagher pushed a commit to jmeagher/pgcat that referenced this pull request Feb 17, 2023
Exit with failure codes if configs are bad (postgresml#146)
Move autoreloader to own tokio task (postgresml#148)
Ruby integration tests (postgresml#147)
Allow running integration tests with coverage locally (postgresml#151)
Log Address information in connection create/drop (postgresml#154)
Better handling for checkout errors during extended protocol messages (postgresml#155)
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