From: | Elvis Pranskevichus <elprans(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Melanie Plageman <melanieplageman(at)gmail(dot)com>, andres(at)anarazel(dot)de, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, daniel(at)yesql(dot)se, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Michael Banck <michael(dot)banck(at)credativ(dot)de> |
Subject: | Re: [HACKERS] [PATCH v2] Add and report the new "session_read_only" GUC pseudo-variable. |
Date: | 2018-11-12 19:29:48 |
Message-ID: | 2901837.nE4RWos2OV@hammer.magicstack.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Monday, November 12, 2018 1:08:37 PM EST Tom Lane wrote:
> Looking through the thread, it seems like there's a pretty fundamental
> design issue that hasn't been resolved, namely whether and how this
> ought to interact with default_transaction_read_only. The patch as
> it stands seems to be trying to implement the definition that the
> transmittable variable session_read_only is equal to
> "!(DefaultXactReadOnly || RecoveryInProgress())". I doubt that
> that's a good idea. In the first place, it's not at all clear that
> such a flag is sufficient for all use-cases. In the second, it's
> somewhere between difficult and impossible to correctly implement
> GUCs that are interdependent in that way; the current patch certainly
> fails to do so. (It will not correctly track intra-session changes
> to DefaultXactReadOnly, for instance.)
>
> I think that we'd be better off maintaining a strict separation
> between "in hot standby" and default_transaction_read_only. If there
> are use cases in which people would like to reject connections based
> on either one being true, let's handle that by marking them both
> GUC_REPORT, and having libpq check both. (So there need to be two
> flavors of target-session-attrs libpq parameter, depending on whether
> you want "in hot standby" or "currently read only" to be checked.)
I agree that the original design with the separate "in_hot_standby" GUC
is better. Hot standby and read-only session are distinct modes, not
all commands that are OK in a read-only session will succeed on a hot-
standby backend.
> I'm also not terribly happy with the patch adding a whole new
> cross-backend signaling mechanism for this; surely we don't really
> need that. Perhaps it'd be sufficient to transmit SIGHUP and embed
> the check for "just exited hot standby" in the handling for that?
That seems doable. Is there an existing check that is a good candidate
for "just exited hot standby"?
Elvis
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2018-11-12 19:35:47 | Re: [HACKERS] Optional message to user when terminating/cancelling backend |
Previous Message | Andrew Dunstan | 2018-11-12 19:08:23 | PostgreSQL BuildFarm Client Release 9 |