From 4b2a5db17bfca262d5c7201dab248769a002d4e1 Mon Sep 17 00:00:00 2001 From: Hari Babu Date: Fri, 22 Feb 2019 00:12:37 +1100 Subject: [PATCH 3/4] Make transaction_read_only as GUC_REPORT varaible transaction_read_only GUC variable value is used in multi host connection to identify the required host of read-write, but currently this carried out by executing a command to find out whether the host is a read-write or not? Instead of that, Reporting the GUC to the client upon connection reduces the time to make the connection. --- doc/src/sgml/libpq.sgml | 15 ++++--- doc/src/sgml/protocol.sgml | 8 ++-- src/backend/utils/misc/guc.c | 2 +- src/interfaces/libpq/fe-connect.c | 70 +++++++++++++++++++++++-------- src/interfaces/libpq/fe-exec.c | 6 ++- src/interfaces/libpq/libpq-int.h | 1 + 6 files changed, 74 insertions(+), 28 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index c1d1b6b2db..5c29beef51 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1581,9 +1581,10 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname connection in which read-write transactions are accepted by default is considered acceptable. The query SHOW transaction_read_only will be sent upon any - successful connection; if it returns on, the connection - will be closed. If multiple hosts were specified in the connection - string, any remaining servers will be tried just as if the connection + successful connection if the server is prior to version 12; if it + returns on, the connection will be closed. + If multiple hosts were specified in the connection string, any + remaining servers will be tried just as if the connection attempt had failed. The default value of this parameter, any, regards all connections as acceptable. @@ -1951,14 +1952,16 @@ const char *PQparameterStatus(const PGconn *conn, const char *paramName); DateStyle, IntervalStyle, TimeZone, - integer_datetimes, and - standard_conforming_strings. + integer_datetimes, + standard_conforming_strings, and + transaction_read_only. (server_encoding, TimeZone, and integer_datetimes were not reported by releases before 8.0; standard_conforming_strings was not reported by releases before 8.1; IntervalStyle was not reported by releases before 8.4; - application_name was not reported by releases before 9.0.) + application_name was not reported by releases before 9.0; + transaction_read_only was not reported by release before 12.0.) Note that server_version, server_encoding and diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index d66b860cbd..6b22dfe47a 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1283,14 +1283,16 @@ SELCT 1/0; DateStyle, IntervalStyle, TimeZone, - integer_datetimes, and - standard_conforming_strings. + integer_datetimes, + standard_conforming_strings, and + transaction_read_only. (server_encoding, TimeZone, and integer_datetimes were not reported by releases before 8.0; standard_conforming_strings was not reported by releases before 8.1; IntervalStyle was not reported by releases before 8.4; - application_name was not reported by releases before 9.0.) + application_name was not reported by releases before 9.0 + transaction_read_only was not reported by releases before 12.0.) Note that server_version, server_encoding and diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 156d147c85..cf62cf8699 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1506,7 +1506,7 @@ static struct config_bool ConfigureNamesBool[] = {"transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the current transaction's read-only status."), NULL, - GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE + GUC_REPORT | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE }, &XactReadOnly, false, diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index e03d78d3f8..25a153f48c 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -3234,26 +3234,61 @@ keep_going: /* We will come back to here until there is if (conn->sversion >= 70400 && conn->requested_session_type == SESSION_TYPE_READ_WRITE) { - /* - * Save existing error messages across the PQsendQuery - * attempt. This is necessary because PQsendQuery is - * going to reset conn->errorMessage, so we would lose - * error messages related to previous hosts we have tried - * and failed to connect to. - */ - if (!saveErrorMessage(conn, &savedMessage)) - goto error_return; - - conn->status = CONNECTION_OK; - if (!PQsendQuery(conn, - "SHOW transaction_read_only")) + if (conn->sversion < 120000) { + /* + * Save existing error messages across the PQsendQuery + * attempt. This is necessary because PQsendQuery is + * going to reset conn->errorMessage, so we would lose + * error messages related to previous hosts we have tried + * and failed to connect to. + */ + if (!saveErrorMessage(conn, &savedMessage)) + goto error_return; + + conn->status = CONNECTION_OK; + if (!PQsendQuery(conn, + "SHOW transaction_read_only")) + { + restoreErrorMessage(conn, &savedMessage); + goto error_return; + } + conn->status = CONNECTION_CHECK_WRITABLE; restoreErrorMessage(conn, &savedMessage); - goto error_return; + return PGRES_POLLING_READING; + } + else if (conn->transaction_read_only) + { + /* Not writable; fail this connection. */ + const char *displayed_host; + const char *displayed_port; + + /* Append error report to conn->errorMessage. */ + if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS) + displayed_host = conn->connhost[conn->whichhost].hostaddr; + else + displayed_host = conn->connhost[conn->whichhost].host; + displayed_port = conn->connhost[conn->whichhost].port; + if (displayed_port == NULL || displayed_port[0] == '\0') + displayed_port = DEF_PGPORT_STR; + + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not make a writable " + "connection to server " + "\"%s:%s\"\n"), + displayed_host, displayed_port); + + /* Close connection politely. */ + conn->status = CONNECTION_OK; + sendTerminateConn(conn); + + /* + * Try next host if any, but we don't want to consider + * additional addresses for this host. + */ + conn->try_next_host = true; + goto keep_going; } - conn->status = CONNECTION_CHECK_WRITABLE; - restoreErrorMessage(conn, &savedMessage); - return PGRES_POLLING_READING; } /* We can release the address list now. */ @@ -3537,6 +3572,7 @@ makeEmptyPGconn(void) conn->setenv_state = SETENV_STATE_IDLE; conn->client_encoding = PG_SQL_ASCII; conn->std_strings = false; /* unless server says differently */ + conn->transaction_read_only = false; conn->verbosity = PQERRORS_DEFAULT; conn->show_context = PQSHOW_CONTEXT_ERRORS; conn->sock = PGINVALID_SOCKET; diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index ac969e7b3f..5adfc36171 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -1033,7 +1033,7 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value) } /* - * Special hacks: remember client_encoding and + * Special hacks: remember client_encoding, transaction_read_only and * standard_conforming_strings, and convert server version to a numeric * form. We keep the first two of these in static variables as well, so * that PQescapeString and PQescapeBytea can behave somewhat sanely (at @@ -1087,6 +1087,10 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value) else conn->sversion = 0; /* unknown */ } + else if (strcmp (name, "transaction_read_only") == 0) + { + conn->transaction_read_only = (strcmp(value, "on") == 0); + } } diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 43aa6b5f30..b0ac98b90d 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -428,6 +428,7 @@ struct pg_conn pgParameterStatus *pstatus; /* ParameterStatus data */ int client_encoding; /* encoding id */ bool std_strings; /* standard_conforming_strings */ + bool transaction_read_only; /* session_read_only */ PGVerbosity verbosity; /* error/notice message verbosity */ PGContextVisibility show_context; /* whether to show CONTEXT field */ PGlobjfuncs *lobjfuncs; /* private state for large-object access fns */ -- 2.20.1.windows.1