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