Re: [HACKERS] [PATCH v2] Add and report the new "session_read_only" GUC pseudo-variable.

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: elprans(at)gmail(dot)com
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, melanieplageman(at)gmail(dot)com, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, daniel(at)yesql(dot)se, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, 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(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-30 16:35:21
Message-ID: CA+q6zcUS=j0oG_RYV3donndEgK5DLN9WXoQQSA05W8qjgF=Obg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Mon, Nov 12, 2018 at 8:30 PM Elvis Pranskevichus <elprans(at)gmail(dot)com> wrote:
>
> 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"?

Apparently due the minor conflict, mentioned above, the patch was in "Waiting
on author" state, which probably is not exactly correct. I'm moving it to the
next CF, but still, it would be nice if you post an updated version without
conflicts.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2018-11-30 16:37:29 Re: test_pg_dump missing cleanup actions
Previous Message Dmitry Dolgov 2018-11-30 16:21:51 Re: [HACKERS] Fix performance degradation of contended LWLock on NUMA