PgConn.CheckConn() does not detect broken connections after v5.4.0

[chacmool]

Describe the bug

We have detected that one of our workloads with pgx v5.4.1 does not handle well database node restarts.
When a node on our Cockroachdb cluster is restarted, first queries on that workload fails with "write: broken pipe".
We have another workload with pgx v5.3.0 using the same database cluster and we don't see these kinds of errors.

We have debugged the issue and we have found that the problem is that PgConn.CheckConn() does not detect a broken connections after v5.4.0.

• pgx/stdlib/sql.go

  Line 473 in d43bd34

  if err := c.conn.PgConn().CheckConn(); err != nil {

Could this commit be related: 3ea2f57

To Reproduce

It is easy to reproduce it locally with this example:

package main

import (
	"database/sql"
	"fmt"
	"log"
	"net/http"
	"os"

	_ "github.com/jackc/pgx/v5/stdlib"
)

func main() {

	db, err := sql.Open("pgx", "postgres://root@localhost:26257/defaultdb?sslmode=disable")
	if err != nil {
		fmt.Fprintf(os.Stderr, "Unable to connect to database: %v\n", err)
		os.Exit(1)
	}
	defer db.Close()

	http.HandleFunc("/hi", func(w http.ResponseWriter, r *http.Request) {
		var greeting string
		err = db.QueryRow("select 'It works!'").Scan(&greeting)
		if err != nil {
			fmt.Fprintf(os.Stderr, "QueryRow failed: %v\n", err)
			fmt.Fprintf(w, "QueryRow failed: %v\n", err)
		}
		fmt.Println(greeting)
		fmt.Fprintf(w, greeting)
	})

	fmt.Printf("Starting server at port 8080\n")
	if err := http.ListenAndServe(":8080", nil); err != nil {
		log.Fatal(err)
	}

}

1. Start a cockroachdb cluster `/usr/local/bin/cockroach start-single-node --insecure --listen-addr=0.0.0.0:26257 --http-addr=:9008`
2. Run the go example
3. Check that http://localhost:8080/hi responds correctly
4. Kill and start again the database cluster `CTLR+C and /usr/local/bin/cockroach start-single-node --insecure --listen-addr=`
5. Refresh the http://localhost:8080/hi page
6a. If the go example uses pgx v5.3.0, it works as before
6b. If the go example uses pgx v5.4.0, "unexpected EOF" error is shown

Expected behavior

Retry a query if the connection used is not longer valid.

Actual behavior

The query fails if the connection used is not longer valid.

Version

• Cockroachdb: 23.1
• pgx: v5.4.0

[jackc]

This is probably related to #1629. Previously, CheckConn did a non-blocking read syscall. However, that system had a number of fundamental issues. Now, it does a normal read with a very short deadline. That should still be sufficient, but perhaps the deadline is triggering before the read is actually attempted. There isn't really any good way to fix CheckConn, as increasing the deadline is the equivalent to adding a sleep. That's why the method is now deprecated.

The solution is might be to change ResetSession to call Ping instead. This will incur latency for a round trip, but it will guarantee that the connection is still live, and it only triggers when the connection has been idle for 1 second.

[ivard]

We also experience undetected broken connections. The following situation occurred in our case:

1. Go application is connected with a database using the following connection string postgresql://user:password@db-a:5432,db-b:5432?sslmode=require&target_session_attrs=read-write where db-a is the primary node.
2. At a certain moment, the connection between the application and db-a freezes (a firewall starts dropping all connections).
3. Issue is detected, a database maintainer starts a failover to db-b.
4. Go application gets stuck in the frozen connection to db-a. Every time a connection to db-a gets a timeout, it first tries to re-connect to db-a. It never gets to the point where it tries to connect to db-b.

We experienced this issue both in 5.3.1 and in 5.4.3. Because of @chacmool's suggestion that 5.3.0 might work better, we also tried that one. It seems to work a little bit better in 5.3.0. After some minutes we see some connections to db-b be made, but it's not very stable. When idle connections are being removed from the database pool or open connections get a timeout, the issue starts again.

[ivard]

We found out that we forgot to set the client_timeout parameter, and the default is to wait indefinitely. So we now think that caused our issue and not this bug.

[jackc]

As of cf6ef75, stdlib uses Ping instead of the deprecated CheckConn. This should resolve the issue. In addition, on a local network it should be faster.