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

[Melanie Plageman]

On Tue, Sep 19, 2017 at 4:15 PM, Melanie Plageman <melanieplage...@gmail.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

Thanks!

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

[Melanie Plageman]

On Tue, Sep 12, 2017 at 6:11 PM, Thomas Munro  wrote:

> On Wed, Sep 13, 2017 at 3:48 AM, Elvis Pranskevichus 
> wrote:
> > I incorporated those bits into your patch and rebased in onto master.
> > Please see attached.
> >
> > FWIW, I think that mixing the standby status and the default
> > transaction writability is suboptimal.  They are related, yes, but not
> > the same thing.  It is possible to have a master cluster in the
> > read-only mode, and with this patch it would be impossible to
> > distinguish from a hot-standby replica without also polling
> > pg_is_in_recovery(), which defeats the purpose of having to do no
> > database roundtrips.
>
> Hi Elvis,
>
> FYI the recovery test 001_stream_rep.pl fails with this patch applied.
> You can see that if you configure with --enable-tap-tests, build and
> then cd into src/test/recovery and "make check".
>
> 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?