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
