Re: Libpq support to connect to standby server as priority

I wrote:
> BTW, I think it would be worth splitting this into separate server-side
> and libpq patches. It looked to me like the server side is pretty
> nearly committable, modulo bikeshedding about the new GUC name.

Actually ... I looked that over again and got a bit more queasy about
all the new signaling logic it is adding. Signals are inherently
bug-prone stuff, plus it's not very clear what sort of guarantees
we'd have about either the reliability or the timeliness of client
notifications about exiting hot-standby mode.

I also wonder what consideration has been given to the performance
implications of marking transaction_read_only as GUC_REPORT, thus
causing client traffic to occur every time it's changed. Most of
the current GUC_REPORT variables don't change too often in typical
sessions, but I'm less convinced about that for transaction_read_only.

So I thought about alternative ways to implement this, and realized
that it would not be hard to make guc.c handle it all by itself, if
we use a custom show-hook for the in_hot_standby GUC that calls
RecoveryInProgress() instead of examining purely static state.
Now, by itself that idea only takes care of the session-start-time
report, because there'd never be any GUC action causing a new
report to occur. But we can improve the situation if we get rid
of the current design whereby ReportGUCOption() is called immediately
when any GUC value changes, and instead batch up the reports to
occur when we're about to go idle waiting for a new client query.

Not incidentally, this responds to a concern Robert mentioned awhile
back about the performance of GUC reporting [1]. You can already get
the server to spam the client excessively if any GUC_REPORT variables
are changed by, for example, functions' SET clauses, because that could
lead to the active value changing many times within a query. We've
gotten away with that so far, but it'd be a problem if any more-often-
changed variables get marked GUC_REPORT. (I actually have a vague
memory of other complaints about that, but I couldn't find any in a
desultory search of the archives.)

So I present 0001 attached which changes the GUC_REPORT code to work
that way, and then 0002 is a pretty small hack to add a reportable
in_hot_standby GUC by having the end-of-query function check (when
needed) to see if the active value changed.

As it stands, 0001 reduces the ParameterStatus message traffic to
at most one per GUC per query, but it doesn't attempt to eliminate
duplicate ParameterStatus messages altogether. We could do that
as a pretty simple adjustment if we're willing to expend the storage
to remember the last value sent to the client. It might be worth
doing, since for example the function-SET-clause case would typically
lead to no net change in the GUC's value by the end of the query.

An objection that could be raised to this approach for in_hot_standby
is that it will only report in_hot_standby becoming false at the end
of a query, whereas the v19 patch at least attempts to deliver an
async ParameterStatus message when the client is idle (and, I think,
indeed may fail to provide *any* message if the transition occurs
when it isn't idle). I don't find that too compelling though;
libpq-based clients, at least, don't have any very practical way to
deal with async ParameterStatus messages anyway.

(Note that I did not touch the docs here, so that while 0001 might
be committable as-is, 0002 is certainly just WIP.)

BTW, as far as the transaction_read_only side of things goes, IMO
it would make a lot more sense to mark default_transaction_read_only
as GUC_REPORT, since that changes a lot less frequently. We'd then
have to expend some work to report that value honestly, since right
now the hot-standby code cheats by ignoring the GUC's value during
hot standby. But I think a technique much like 0002's would work
for that.

Thoughts?

regards, tom lane

[1] https://www.postgresql.org/message-id/CA%2BTgmoaDoVtMnfKNFm-iyyCSp%3DFPiHkfU1AXuEHJqmcLTAX6kQ%40mail.gmail.com