Re: Libpq support to connect to standby server as priority

On Mon, Jan 14, 2019 at 5:17 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> The problem here of course is that whoever invented target_session_attrs
> was unconcerned with following that precedent, so what we have is
> "target_session_attrs=(any | read-write)".
> Are we prepared to add some aliases in service of unifying these names?

I wasn't unconcerned about the problem, but I wasn't prepared to to be
the first person who added a connection parameter that used
namesLikeThis instead of names_like_this, especially if the semantics
weren't exactly the same. That seemed to be a recipe for somebody
yelling at me, and I try to avoid that when I can.

> 4. Given that other discussion, it's not quite clear what we should
> even be checking. The existing logic devolves to checking that
> transaction_read_only is true, but that's not really the same thing as
> "is a master server", eg you might have connected to a master server
> under a role that has SET ROLE default_transaction_read_only = false.
> (I wonder what pgJDBC is really checking, under the hood.)
> Do we want to have modes that are checking hot-standby state in some
> fashion, rather than the transaction_read_only state?

Well, this has been discussed before, too, I'm pretty sure, but I'm
too lazy to go find the old discussion right now. The upshot is that
default_transaction_read_only lets an administrator make a server look
read-only even if it technically isn't, which somebody might find
useful. Otherwise what do you do if, for example, you are using
logical replication? None of your servers are in recovery, but you
can make some of them report default_transaction_read_only = true if
you like. To me, that kind of configurability is a feature, not a
bug.

That being said, I don't object to having even more values for
target_session_attrs that check other things. You could have:

read_only: default_transaction_read_only => true
read_write: default_transaction_read_only => false
master: pg_is_in_recovery => false
standby: pg_is_in_recovery => true

But what I think would be a Very Bad Plan is to use confused naming
that looks for something different than what it purports to do. For
example, if you were to change things so that read_write checks
pg_is_in_recovery(), then you might ask for a "read-write" server and
get one where only read-only transactions are permitted. We need not
assume that "read-write master" and "read-only standby" are the only
two kinds of things that can ever exist, as long as we're careful
about the names we choose. Choosing the names carefully also helps to
avoid POLA violations.

Another point I'd like to mention is that target_session_attrs could
be extended to care about other kinds of properties which someone
might want a server to have, quite apart from
master/standby/read-only/read-write. I don't know exactly what sort
of thing somebody might care about, but the name is such that we can
decide to care about other properties in the future without having to
add a whole new parameter. You can imagine a day when someone can say
target_session_attrs=read-write,v42+,ftl to get a server connection
that is read-write on a server running PostgreSQL 42 or greater that
also has a built-in hyperdrive. Or whatever.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company