server,sql: complement query_wait with conn_wait to wait until clients/pool closes connections

[lancel66]

Customer needs a truly hitless way of doing upgrades. Current behavior is to:

1. Push new binary to temp
2. Drain node
3. Install/replace binary
4. Restart crdb

Drain 1) stops accepting client connections, then 2) stops extant connections. The problem is that stopping existing connections causes issues with workloads when they are not retried.

They would like a hitless way to do rolling inplace upgrades.

One thought is to have a flag to tell drain to wait out connections for a definable period of time, rather than closing connections.

cc @rafiss

gz#8424

Epic CRDB-10458

Jira issue: CRDB-7990

[blathers-crl]

Hello, I am Blathers. I am here to help you get the issue triaged.

It looks like you have not filled out the issue in the format of any of our templates. To best assist you, we advise you to use one of these templates.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

• Join our community slack channel and ask on #cockroachdb.
• Try find someone from here if you know they worked closely on the area and CC them.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[rafiss]

I'd like to understand a bit more

One thought is to have a flag to tell drain to wait out connections for a definable period of time

Is the concern that any queries that are currently running are getting stopped? Have they already tried using the server.shutdown.query_wait cluster setting?

rather than closing connections.

At some point, the connection does need to get closed. I don't see a way around that. Is the customer connecting using a connection pool and load balancer? If so, then two points:

1. The load balancer should be checking the health of the CRDB nodes. It should be doing this by checking health on the /health?ready=1 endpoint. See more docs. If the load balancer is already checking that endpoint, but still is not removing CRDB nodes that are shutting down in a timely fashion, then they may need to increase the server.shutdown.drain_wait setting.
2. The connection pool should be checking the health of a connection before trying to use it. If a CRDB node is in the process of draining, then the connection pool should see that, and prevent the application from using the connection. The connection pool will then try again and give the application a connection that is healthy. See more docs, though it would be most helpful for them to review the docs of the connection pool they are using.

---

@lancel66 Based on all the above, if there are errors happening during upgrades, it sounds more like a bug report than a feature request. I recommend filing this through normal support channels. If you do so, it will also be very helpful to include more specific error messages and details on their load balancer, connection pool, and application.

[rafiss]

I've thought about this a bit more:

It seems like the concern is with the fact that cockroach closes connections on the server-side during the shutdown process. Some other systems don’t do this — they require all the clients to close the connections and only then will the server process let itself terminate.

The way crdb closes connections on the server-side could be a problem, since the client could be in the middle of using the connection when crdb decides to close it. (Perhaps even more common when a connection pool is in the mix, as we've seen with other scenarios; see internal tickets https://github.com/cockroachlabs/support/issues/984 and https://cockroachlabs.atlassian.net/browse/CC-2756)

It seems like we might want to offer one more knob similar to server.shutdown.query_wait — except instead of waiting for queries to finish, it should wait for connections to be terminated on the client side. It could be named something like server.shutdown.connection_wait.

So the order of waiting would be:

1. drain_wait: wait for load balancer to notice that instances are shutting down
2. connection_wait: wait for clients to close connections (could be set to "infinity" to satisfy the request in this ticket)
3. query_wait: wait longer in case there are still queries in flight
4. lease_transfer_wait: wait for leases to move to new nodes after now that all queries are done

Also, instead of a cluster setting, this might work as a flag to the drain command, as the initial feature request points out.