Re: Libpq support to connect to standby server as priority

On Wed, Jan 6, 2021 at 3:05 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Greg Nancarrow <gregn4422@gmail.com> writes:
> > Posting an updated set of patches.
>
> I've reviewed and pushed most of v20-0001, with the following changes:
>
> * I realized that we had more moving parts than necessary for
> in_hot_standby.  We don't really need two static variables, one is
> sufficient --- and we shouldn't make the SHOW hook have side-effects,
> that's just dangerous.
>
> * The documentation patches were missing an addition to config.sgml,
> as well as failing to list the new GUC in the two places where we
> document all GUC_REPORT variables.
>
> What I did *not* push was the change to mark transaction_read_only
> as GUC_REPORT.  I find that idea highly dubious, for a couple of
> reasons:
>
> * It'll create useless ParameterStatus traffic during normal operations
> of an application using "START TRANSACTION READ ONLY" or the like.
>
> * I do not think it will actually work for the desired purpose of
> finding out the read-only state during session connection.  AFAICS,
> we don't set XactReadOnly to a meaningful value except when starting
> a transaction.  Yeah, we'll run a transaction during login because
> we have to examine the system catalogs ... but do we start a new
> one after absorbing the effects of, say, ALTER USER SET
> default_transaction_read_only?  I doubt it, and even if it works
> today it'd be fragile, because someday somebody will try to collapse
> any multiple transactions during startup into one transaction.
>
> I think what we want to do is mark default_transaction_read_only as
> GUC_REPORT, instead.  That will give a reliable report of what the
> state of its GUC is, and you can combine it with is_hot_standby
> to decide whether the session should be considered read-only.
> If you don't get those two GUC values during connection, then you
> can fall back on "SHOW transaction_read_only".
>

I have made a patch for the above with the changes suggested and
rebased it with the head code.
Attached v21 patch which has the changes for the same.
Thoughts?

Regards,
Vignesh