From 1eaf6bd08575935e223569e1c4fb8dcf19439246 Mon Sep 17 00:00:00 2001 From: Hari Babu Date: Wed, 27 Mar 2019 17:05:24 +1100 Subject: [PATCH 3/8] Make transaction_read_only as GUC_REPORT variable 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 | 14 ++++--- 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(+), 27 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 17310eb077..6107755416 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1665,8 +1665,10 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname To find out whether the server supports read-write transactions are not, query SHOW transaction_read_only will be sent upon any - successful connection; if it returns on, it means server - doesn't support read-write transactions. + successful connection if the server is prior to version 12; if it returns + on, it means server doesn't support read-write transactions. + But for server version 12 or greater uses the value of transaction_read_only + configuration parameter that is reported by the server upon successful connection. @@ -2032,14 +2034,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 b20f1690a7..05286c7365 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1283,14 +1283,16 @@ SELECT 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 92c4fee8f8..5357e51d3f 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1544,7 +1544,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 8b2d46d31c..eceb586fb2 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -3483,26 +3483,61 @@ keep_going: /* We will come back to here until there is if (conn->sversion >= 70400 && conn->requested_session_type != SESSION_TYPE_ANY) { - /* - * 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. */ @@ -3783,6 +3818,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 3a8cddf4ff..701c4a66fe 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -1059,7 +1059,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 @@ -1113,6 +1113,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 4f0d20a696..caddf0ddc5 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -431,6 +431,7 @@ struct pg_conn pgParameterStatus *pstatus; /* ParameterStatus data */ int client_encoding; /* encoding id */ bool std_strings; /* standard_conforming_strings */ + bool transaction_read_only; /* transaction_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.21.0