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

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Melanie Plageman <melanieplageman(at)gmail(dot)com>
Cc: Elvis Pranskevichus <elprans(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Michael Banck <michael(dot)banck(at)credativ(dot)de>
Subject: Re: [PATCH v1] Add and report the new "in_hot_standby" GUC pseudo-variable.
Date: 2017-10-02 07:16:23
Message-ID: 5C470924-91C3-49DB-852A-95924B60B6F5@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 22 Sep 2017, at 18:57, Melanie Plageman <melanieplageman(at)gmail(dot)com> wrote:
>
> On Tue, Sep 19, 2017 at 4:15 PM, Melanie Plageman <melanieplageman(at)gmail(dot)com <mailto:melanieplageman(at)gmail(dot)com>> wrote:
> The latest patch applies cleanly and builds (I am also seeing the failing TAP tests), however, I have a concern. With a single server set up, when I attempt to make a connection with target_session_attrs=read-write, I get the message
> psql: could not make a suitable connection to server "localhost:5432"
> Whereas, when I attempt to make a connection with target_session_attrs=read-only, it is successful.
>
> I might be missing something, but this seems somewhat counter-intuitive. I would expect to specify read-write as target_session_attrs and successfully connect to a server on which read and write operations are permitted. I see this behavior implemented in src/interfaces/libpq/fe-connect.c
> Is there a reason to reject a connection to a primary server when I specify 'read-write'? Is this intentional?
>
> Hi Elvis,
>
> Making an assumption about the intended functionality mentioned above, I swapped the 'not' to the following lines of src/interfaces/libpq/fe-connect.c ~ line 3005
>
> if (conn->target_session_attrs != NULL &&
> ((strcmp(conn->target_session_attrs, "read-write") == 0 && conn->session_read_only) ||
> (strcmp(conn->target_session_attrs, "read-only") == 0 && !conn->session_read_only)))
>
> I rebased and built with this change locally.
> The review below is based on the patch with that change.
>
> Also, the following comment has what looks like a copy-paste error and the first line should be deleted
> in src/backend/utils/misc/guc.c ~ line 10078
> assign_default_transaction_read_only
>
>
> +assign_default_transaction_read_only(bool newval, void *extra)
> ...
> + /*
> - * We clamp manually-set values to at least 1MB. Since
> + * Also set the session read-only parameter. We only need
> + * to set the correct value in processes that have database
> + * sessions, but there's no mechanism to know that there's
>
> patch applies cleanly: yes
> installcheck: passed
> installcheck-world: passed
> feature works as expected: yes (details follow)
>
> With two servers, one configured as the primary and one configured to run in Hot Standby mode, I was able to observe that the value of session_read_only changed after triggering failover once the standby server exited recovery
>
> When attempting to connect to a primary server with target_session_attrs=read-write, I was successful and when attempting to connect with target_session_attrs=read-only, the connection was closed and the expected message was produced

Based on the unaddressed questions raised in this thread, I’m marking this
patch Returned with Feedback. Please re-submit a new version of the patch to a
future commitfest.

cheers ./daniel

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2017-10-02 07:28:20 Re: document and use SPI_result_code_string()
Previous Message Masahiko Sawada 2017-10-02 06:31:34 Re: Transactions involving multiple postgres foreign servers