From: Tom Lane [mailto:t...@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




Reply via email to