RE: Libpq support to connect to standby server as priority
Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>
From: "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>
To: 'Tom Lane' <tgl@sss.pgh.pa.us>, Dave Cramer <pg@fastcrypt.com>
Cc: Robert Haas <robertmhaas@gmail.com>, "laurenz.albe@cybertec.at" <laurenz.albe@cybertec.at>, Haribabu Kommi <kommi.haribabu@gmail.com>, "Jing
Wang" <jingwangian@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2019-01-15T02:00:57Z
Lists: pgsql-hackers
From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> 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 think "yes".
> 2. Whether or not you want to follow pgJDBC's naming, it seems like we
> ought to have both "require read only" and "prefer read only" behaviors
> in this patch, and maybe likewise "require read write" versus "prefer
> read write".
Agreed, although I don't see a use case for "prefer read write". I don't think there's an app like "I want to write, but I'm OK if I cannot."
> 3. We ought to sync this up with whatever's going to happen in
> https://commitfest.postgresql.org/21/1090/
> at least to the extent of agreeing on what GUCs we'd like to see
> the server start reporting.
Yes.
> 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?
PgJDBC uses transaction_read_only like this:
[core/v3/ConnectionFactoryImpl.java]
private boolean isMaster(QueryExecutor queryExecutor) throws SQLException, IOException {
byte[][] results = SetupQueryRunner.run(queryExecutor, "show transaction_read_only", true);
String value = queryExecutor.getEncoding().decode(results[0]);
return value.equalsIgnoreCase("off");
}
But as some people said, I don't think this is the right way. I suppose what's leading to the current somewhat complicated situation is that there was no easy way for the client to know whether the server is the master. That ended up in using "SHOW transaction_read_only" instead, and people supported that compromise by saying "read only status is more useful than whether the server is standby or not," I'm afraid.
The original desire should have been the ability to connect to a primary or a standby. So, I think we should go back to the original thinking (and not complicate the feature), and create a read only GUC_REPORT variable, say, server_role, that identifies whether the server is a primary or a standby.
Regards
Takayuki Tsunakawa
Commits
-
Make test_target_session_attrs more robust against connection failure.
- 3769e11a3183 14.0 landed
-
Extend the abilities of libpq's target_session_attrs parameter.
- ee28cacf619f 14.0 landed
-
Mark default_transaction_read_only as GUC_REPORT.
- d16f8c8e416d 14.0 landed
-
Introduce a new GUC_REPORT setting "in_hot_standby".
- bf8a662c9afa 14.0 landed
-
Avoid spamming the client with multiple ParameterStatus messages.
- 2432b1a04087 14.0 landed
-
Avoid harmless Valgrind no-buffer-pin errors.
- a766d6ca22ac 14.0 cited
-
Restructure libpq code to remove some duplicity
- b438e7e7a1c5 13.0 landed
-
libpq: Add target_session_attrs parameter.
- 721f7bd3cbcc 10.0 cited