On 11 April 2017 at 09:05, Magnus Hagander <magnus@hagander.net> wrote: > On Tue, Apr 11, 2017 at 3:26 AM, Michael Paquier <michael.paquier@gmail.com> > wrote: >> >> On Mon, Apr 10, 2017 at 5:47 PM, Magnus Hagander <magnus@hagander.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.
