perf: read in_hot_standby GUC on connection (#2334)

* perf: read in_hot_standby GUC on connection

* use static const for in_hot_standby

pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java

@@ -76,6 +76,8 @@ public class ConnectionFactoryImpl extends ConnectionFactory {
   private static final int AUTH_REQ_SASL_CONTINUE = 11;
   private static final int AUTH_REQ_SASL_FINAL = 12;
 
+  private static final String IN_HOT_STANDBY = "in_hot_standby";
+
   private ISSPIClient createSSPI(PGStream pgStream,
       @Nullable String spnServiceClass,
       boolean enableNegotiate) {
@@ -863,10 +865,44 @@ private void runInitialQueries(QueryExecutor queryExecutor, Properties info)
     }
   }
 
+  /**
+   * Since PG14 there is GUC_REPORT ParamStatus {@code in_hot_standby} which is set to "on"
+   * when the server is in archive recovery or standby mode. In driver's lingo such server is called
+   * {@link org.postgresql.hostchooser.HostRequirement#secondary}.
+   * Previously {@code transaction_read_only} was used as a workable substitute.
+   * However {@code transaction_read_only} could have been manually overridden on the primary server
+   * by database user leading to a false positives: ie server is effectively read-only but
+   * technically is "primary" (not in a recovery/standby mode).
+   *
+   * <p>This method checks whether {@code in_hot_standby} GUC was reported by the server
+   * during initial connection:</p>
+   *
+   * <ul>
+   * <li>{@code in_hot_standby} was reported and the value was "on" then the server is a replica
+   * and database is read-only by definition, false is returned.</li>
+   * <li>{@code in_hot_standby} was reported and the value was "off"
+   * then the server is indeed primary but database may be in
+   * read-only mode nevertheless. We proceed to conservatively {@code show transaction_read_only}
+   * since users may not be expecting a readonly connection for {@code targetServerType=primary}</li>
+   * <li>If {@code in_hot_standby} has not been reported we fallback to pre v14 behavior.</li>
+   * </ul>
+   *
+   * <p>Do not confuse {@code hot_standby} and {@code in_hot_standby} ParamStatuses</p>
+   *
+   * @see <a href="https://www.postgresql.org/docs/current/protocol-flow.html#PROTOCOL-ASYNC">GUC_REPORT documentation</a>
+   * @see <a href="https://www.postgresql.org/docs/current/hot-standby.html">Hot standby documentation</a>
+   * @see <a href="https://www.postgresql.org/message-id/flat/1700970.cRWpxnom9y@hammer.magicstack.net">in_hot_standby patch thread v10</a>
+   * @see <a href="https://www.postgresql.org/message-id/flat/CAF3%2BxM%2B8-ztOkaV9gHiJ3wfgENTq97QcjXQt%2BrbFQ6F7oNzt9A%40mail.gmail.com">in_hot_standby patch thread v14</a>
+   *
+   */
   private boolean isPrimary(QueryExecutor queryExecutor) throws SQLException, IOException {
+    String inHotStandby = queryExecutor.getParameterStatus(IN_HOT_STANDBY);
+    if ("on".equalsIgnoreCase(inHotStandby)) {
+      return false;
+    }
     Tuple results = SetupQueryRunner.run(queryExecutor, "show transaction_read_only", true);
     Tuple nonNullResults = castNonNull(results);
-    String value = queryExecutor.getEncoding().decode(castNonNull(nonNullResults.get(0)));
-    return value.equalsIgnoreCase("off");
+    String queriedTransactionReadonly = queryExecutor.getEncoding().decode(castNonNull(nonNullResults.get(0)));
+    return queriedTransactionReadonly.equalsIgnoreCase("off");
   }
 }

pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java

@@ -99,6 +99,7 @@ public class QueryExecutorImpl extends QueryExecutorBase {
     Encoding.canonicalize("TimeZone");
     Encoding.canonicalize("UTF8");
     Encoding.canonicalize("UTF-8");
+    Encoding.canonicalize("in_hot_standby");
   }
 
   /**

pgjdbc/src/test/java/org/postgresql/test/hostchooser/MultiHostsConnectionTest.java

@@ -27,6 +27,7 @@
 import org.postgresql.test.TestUtil;
 import org.postgresql.util.HostSpec;
 import org.postgresql.util.PSQLException;
+import org.postgresql.util.PSQLState;
 
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -236,6 +237,29 @@ public void testConnectToMaster() throws SQLException {
     assertGlobalState(secondary1, "Secondary"); // was unknown, so tried to connect in order
   }
 
+  @Test
+  public void testConnectToPrimaryWithReadonlyTransactionMode() throws SQLException {
+    con = TestUtil.openPrivilegedDB();
+    con.createStatement().execute("ALTER DATABASE " + TestUtil.getDatabase() + " SET default_transaction_read_only=on;");
+    try {
+      getConnection(primary, true, fake1, primary1, secondary1);
+    } catch (PSQLException e) {
+      assertEquals(PSQLState.CONNECTION_UNABLE_TO_CONNECT.getState(), e.getSQLState());
+      assertGlobalState(fake1, "ConnectFail");
+      assertGlobalState(primary1, "Secondary");
+      assertGlobalState(secondary1, "Secondary");
+    } finally {
+      con = TestUtil.openPrivilegedDB();
+      con.createStatement().execute(
+          "BEGIN;"
+          + "SET TRANSACTION READ WRITE;"
+          + "ALTER DATABASE " + TestUtil.getDatabase() + " SET default_transaction_read_only=off;"
+          + "COMMIT;"
+      );
+      TestUtil.closeDB(con);
+    }
+  }
+
   @Test
   public void testConnectToSecondary() throws SQLException {
     getConnection(secondary, true, fake1, secondary1, primary1);
@@ -347,7 +371,7 @@ public void testLoadBalancing_secondary() throws SQLException {
         break;
       }
     }
-    assertEquals("Did not attempt to connect to all salve hosts", new HashSet<String>(asList(secondaryIP, secondaryIP2)),
+    assertEquals("Did not attempt to connect to all secondary hosts", new HashSet<String>(asList(secondaryIP, secondaryIP2)),
         connectedHosts);
     assertEquals("Did not attempt to connect to primary and fake node", 4, tryConnectedHosts.size());