Re: Add GoAway protocol message for graceful but fast server shutdown/switchover

Hi,

I've looked at this patch today, to check if there's something we could
get done in PG19.

I find it a bit we didn't get much feedback from people working on the
client/downstream stuff - clients, connection poolers/middleware, that
sort of stuff. OK, we did hear from Kiril, and he seems to like it.

I'm not very involved in the protocol stuff, so I'm sure there's a lot
details I'm missing. It'd be very helpful if there was some sort of PoC
support on the pooler/client side, so that I can experiment with it and
see how helpful the new protocol message is. But I realize that's a bit
too much to ask for.

A couple thoughts about this (some of this may be missing what the patch
aims to do).

* Does it make sense to tie this to smart shutdowns? I realize it's just
an example, and it probably makes sense to send the GoAway message
before a shutdown. But isn't this a bit similar to cancel/terminate of a
backend? Why not to have a pg_goaway_backend() function, that'd send the
message to a single backend? It might be useful for load-balancing, if
we could pick a "heavy" backend and ask it to reconnect / move to a
different replica. (Could that be handled by a middleware?)

* In fact, does it improve the smart shutdown case in practice? Let's
say we have a single instance, and we're restarting it. It'll send
GoAway to all the clients, the good clients will try to reconnect. But
if there's even a single "bad" client ignoring the GoAway, all the
well-behaved clients will get stuck. Ofc, that can happen without the
GoAway message too - a client may disconnect because of timeout etc. But
it makes it more likely, and it'll affect the well-behaved clients.

* Would it make sense to have some payload in the GoAway message? I'm
thinking about (a) some deadline by which the client should disconnect,
e.g. time of planned restart / shutdown, (b) priority, expressing how
much the client should try to disconnect (and maybe take more drastic
actions).

Also, two minor comments:

* The sgml docs say the function is defined as

int PQgoAwayReceived(const PGconn *conn);

but in the .h file it's defined without the "const".

* The new entry in protocol.sgml (in the "Supported Protocol Extensions"
table) says

<entry><literal>goaway</literal></entry>

but the following table includes "_pq_" in the entry name. Should the
new entry do the same?

regards

--
Tomas Vondra