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

From: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
To: 'Robert Haas' <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Elvis Pranskevichus <elprans(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH v1] Add and report the new "in_hot_standby" GUC pseudo-variable.
Date: 2017-04-13 06:59:59
Message-ID: 0A3221C70F24FB45833433255569204D1F6C7135@G01JPEXMBYT05
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello,

I didn't realize that my target_session_attrs naming proposal was committed. I didn't think half way it would be adopted, because the name is less intuitive than the original target_server_type, and is different from the PgJDBC's targetServerType.

From: pgsql-hackers-owner(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Simon Riggs
> 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

I totally agree. I'm a bit disappointed with and worried about the current situation. I could easily imagine that people around me would say a stern opinion on the specification...

I think these are necessary in descending order of priority, if based on target_session_attrs:

[PG10]
1. target_session_attrs=read-only
This is mainly to connect to the standby. People will naturally expect that this is available, because PostgreSQL provides hot standby feature, and other DBMSs and even PgJDBC has the functionality.

2. Make transaction_read_only GUC_REPORT
This is to avoid the added round-trip by SHOW command. It also benefits client apps that want to know when the server gets promoted? And this may simplify the libpq code.
I don't understand yet why we need to provide this feature for older servers by using SHOW. Those who are already using <= 9.6 in production completed the system or application, and their business is running. Why would they want to just replace libpq and use this feature?

[PG 11]
3. target_session_attrs=prefer-read-only
This is mainly to prefer standbys, but allows to connect to the primary if no standby is available. Honestly, this is also required in PG 10 because PgJDBC already provides this by "preferSlave".

From: pgsql-hackers-owner(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Robert Haas
> On Tue, Apr 11, 2017 at 4:05 AM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > 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 don't really agree. I think if we go and install a GUC_REPORT GUC now,
> we're much less likely to flush out the bugs in the 'show
> transaction_read_only' mechanism.

I'm sorry I couldn't get this part (maybe purely English nuance. Are you concerned about some bugs? We can't do anything if we fear of bugs. Is it OK instead to make transaction_read_only GUC_REPORT?

> Also, now that I think about, the reason
> why we settled on 'show transaction_read_only' against alternate queries
> is because there's some ability for the DBA to make that return 'false'
> rather than 'true' even when not in recovery, so that if for example you
> are using logical replication rather than physical replication, you have
> a way to make target_session_attrs=read-write still do something useful.
> If you add an in_hot_standby GUC that's used instead, you lose that.

Agreed. Again, is this satisfied by GUC_REPORTing transaction_read_only as well?

> Now, we can decide what we want to do about that, but I don't see that a
> change in this area *must* go into v10. Maybe the answer is that
> target_session_attrs grows additional values like 'primary' and 'standby'
> alongside 'read-write' (and Simon's suggested 'read-only').
> Or maybe we have another idea. But I don't really see the urgency of
> whacking this around right this minute. There's nothing broken here;
> there's just more stuff people would like to have. It can be added next
> time around.

But if completeness of the functionality is below people's expectations, it may unnecessarily compromise the reputation of PostgreSQL.

Is there any chance to incorporate a patch into PG 10? May I add this as a PG 10 open item?

Regards
Takayuki Tsunakawa

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Borodin 2017-04-13 07:01:46 Re: Merge join for GiST
Previous Message Jeff Davis 2017-04-13 06:30:33 Re: Merge join for GiST