Re: [PATCH v1] Add and report the new "in_hot_standby" GUC pseudo-variable.

On Tue, Apr 11, 2017 at 2:38 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:

> On 11 April 2017 at 09:05, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> > On Tue, Apr 11, 2017 at 3:26 AM, Michael Paquier <
> michael(dot)paquier(at)gmail(dot)com>
> > wrote:
> >>
> >> On Mon, Apr 10, 2017 at 5:47 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> >> wrote:
> >> > Based on that we seem to agree here, should we add this as an open
> item?
> >> > Clearly if we want to change this, we should do so before 10.
> >>
> >> This really is a new feature, so as the focus is to stabilize things I
> >> think that we should not make the code more complicated because...
> >
> >
> > The part I'm talking about is the potential adjustment of the patch
> that's
> > already committed. That's not a new feature, that's exactly the sort of
> > thing we'd want to adjust before we get to release. Because once
> released we
> > really can't change it.
>
> I agree if we introduce target_session_attrs it would be better to
> have a complete feature in one release.
>
> It does seem quite strange to have
> target_session_attrs=read-write
> but not
> target_session_attrs=read-only
>
> And it would be even better to have these session attrs as well
> notify-on-promote - sent when standby is promoted
> notify-on-write - sent when an xid is assigned
>

Well, one of those could come automatically with a GUC_REPORT variable of
the correct type, no? So if we were to use the transaction_read_only one,
you'd get a notification on promotion because your transaction became
read/write, wouldn't it?

>
> "notify-on-promotion" being my suggested name for the feature being
> discussed here. In terms of the feature as submitted, I wonder whether
> having a GUC parameter like this makes sense, but I think its ok for
> us to send a protocol message, maybe a NotificationResponse, but there
> isn't any material difference between those two protocol messages.
>
> Rather than the special case code in the patch, I imagine more generic
> code like this...
>
> if (sessionInterruptPending)
> ProcessSessionInterrupt();
>
> I'm happy to work on the patch, if that's OK.
>

I think going through all those steps is moving the goalposts a bit too far
for the 10 release.

But if adjustment to the already applied patch is needed to make sure we
can improve on it to get to this point in 11, that's more on topic I think.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>