Query timeouts closes underlying connections

[ericreis]

Hello! I was wondering if anyone can help me understand a strange behavior when using pgx@v4. When a query times out it's automatically closed. This is causing the DialFunc to be called again to recreate the connection, and this is a costly operation, especially when using a big connection pool. From time to time my metrics show there's peek in dial calls (sometimes up to 70 calls per second) which causes them to timeout. This creates a snowball because, whenever a timeout occurs, the connection is closed and it has to be reopened, triggering more dial calls. Am I doing something wrong?

I've seen in this CHANGELOG that now we can Allow background pool connections to continue even if cause is canceled (James Hartig), but I haven't managed to get that working.

Browsing the code, I came across this piece of code that is closing the connection if any error occurs. In other functions, net timeout errors are explicitly ignored, as it can be seen here, and also here. Should this types of error also be ignored in the Prepare function?

[jackc]

There's multiple issues you reference

One is the pool establishing a connection not getting interrupted when the query that triggered the connection is interrupted. AFAIK this is working properly. So if that is not happening there is a bug somewhere. But you didn't say if you are using database/sql or pgxpool. This change only affects pgxpool. I don't know if database/sql is affected or not.

The second issue is closing the connection on (almost) any error. The reason for this is it is extremely difficult to reliably recover from an interrupted connection. There are so many different edge cases that it is better to reestablish a new connection. The places that allow timeouts are to support interrupting WaitForNotification().

[ericreis]

@jackc Sorry if I missed some points. I am using pgxpool and the behavior I am getting is the following: I make a query that is canceled by its context. When it happens, the connection is automatically closed even though it wasn't an actual problem with the connection. This is making my application create a bunch of connections when this timeout happens. From what I understood from the v4.17 changelog I should be able to reuse that connection. Right?

[jackc]

I don't think a callback would work -- or at least it would be a significant challenge. Even the simplest case is complicated.

The simplest case would be a query is sent and the context is cancelled before the any response is received. Cancelling a query has aspects that make this tricky.

1. A context cancellation needs to take effect immediately. This means there will be unread messages sent from the server. These need to be read before the connection can be used again. We may or may not know how many messages we need to read.
2. The query needs to be cancelled on the server. We don't want a long running query wasting server resources when the client isn't listening anymore. And if we did have some sort of recover / resync process we don't want to wait for the query to finish.
3. Telling the server to cancel a running query is an async operation that does not report on success or status. It is performed by establishing a new connection to the server and sending a special command. Our cancel request may only cancel one query when multiple are queued. Or the original query may finish, the application sends another query on that connection, and then the cancel request cancels that query.

---

There are alternatives that can be implemented application side.

If you don't mind your Query() blocking for a few seconds after a context cancellation you could have your context cancellation trigger a https://pkg.go.dev/github.com/jackc/pgconn#PgConn.CancelRequest call. Your actual Query() call would use a different context. In most cases your Query() would return an error in under a second, but if it didn't then you would cancel the other context.

But if a context cancellation must cancel the Query() call immediately then you would also need to run the query in a goroutine.

[ericreis]

Got it! I think this is not the intended behavior we want also. I think there'll be no other way to deal with this but to optimize our connections resource with a different approach (batch or pipeline). Thank you so much for the responses and the suggestions!

[craigpastro]

Hello 👋 Sorry for butting in here, but I am currently facing the problem described above: the context gets cancelled and leads to dropped connections. If I expect that my queries will be under 1s do you think is it better to use a fresh context and let the queries run their course, rather than worry about dropped connections and having to reconnect?

Just for fun I tried wrapping the queries in a transaction and noticed that connections were not being dropped. I didn't dig into the code yet, but I assume this has something to do with the WaitForNotification() you allude to above? This was a lie. Please ignore.

Thank you very much!

[jackc]

@craigpastro Yes, it probably makes sense to try to wait a little while before cancelling the query if you expect it to be a short wait.

[craigpastro]

Thanks @jackc. I'll try that out.