From 74cad19872e70ffe937ad582fab30bb8f17f7a16 Mon Sep 17 00:00:00 2001 From: Greg Nancarrow Date: Mon, 30 Sep 2019 11:55:57 +1000 Subject: [PATCH v14 1/3] Enhance libpq target_session_attrs: read-write/prefer-read/read-only Improve checking of the requested target session type, to avoid always having to do string comparisons on target_session_attrs. Make "transaction_read_only" a GUC_REPORT variable, to avoid having to execute a query post-connection in order to determine whether a host is read-write (and reduce time to make the connection). Add new "prefer-read" target_session_attrs option value, to support connecting to a read-only server if available from the list of hosts (otherwise connect to a read-write server). Add new "read-only" target_session_attrs option value, to support connecting to a read-only server if available from the list of hosts (otherwise the connection attempt fails). Discussion: https://www.postgresql.org/message-id/flat/CAF3+xM+8-ztOkaV9gHiJ3wfgENTq97QcjXQt+rbFQ6F7oNzt9A@mail.gmail.com --- doc/src/sgml/libpq.sgml | 54 ++++++-- doc/src/sgml/protocol.sgml | 8 +- src/backend/utils/misc/guc.c | 2 +- src/interfaces/libpq/fe-connect.c | 239 +++++++++++++++++++++++++++++----- src/interfaces/libpq/fe-exec.c | 6 +- src/interfaces/libpq/libpq-fe.h | 8 ++ src/interfaces/libpq/libpq-int.h | 15 ++- src/test/recovery/t/001_stream_rep.pl | 22 +++- 8 files changed, 299 insertions(+), 55 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index c58527b..0d3edfc 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1674,18 +1674,46 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname target_session_attrs + The supported options for this parameter are any, + read-write, prefer-read and read-only. + The default value of this parameter, any, regards + all connections as acceptable. If multiple hosts are specified in the + connection string, each host is tried in the order given until a connection + is successful. + + + If this parameter is set to read-write, only a 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 - attempt had failed. The default value of this parameter, - any, regards all connections as acceptable. - + is considered acceptable. + + + + If this parameter is set to prefer-read, a + connection in which read-only transactions are accepted by default + is preferred. If no such connections can be found, then a connection + in which read-write transactions are accepted will be considered. + + + + To determine whether the server supports read-write transactions, the + query SHOW transaction_read_only will be sent upon any + successful connection, if the server is prior to version 13; if it returns + on, it means the server doesn't support read-write + transactions. + If the server is version 13 or greater, the support of read-write + transactions is determined by the value of the + transaction_read_only configuration parameter that is + reported by the server upon successful connection. + + + + If this parameter is set to read-only, only a connection + in which read-only transactions are accepted by default is considered + acceptable. + - + @@ -1993,14 +2021,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 releases before 13.0.) Note that server_version, server_encoding and diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 8027521..87b95bc 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 13.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 2178e1c..c64ec03 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1543,7 +1543,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 f91f0f2..7058f5d 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -350,7 +350,7 @@ static const internalPQconninfoOption PQconninfoOptions[] = { {"target_session_attrs", "PGTARGETSESSIONATTRS", DefaultTargetSessionAttrs, NULL, - "Target-Session-Attrs", "", 11, /* sizeof("read-write") = 11 */ + "Target-Session-Attrs", "", 12, /* sizeof("prefer-read") = 12 */ offsetof(struct pg_conn, target_session_attrs)}, /* Terminating entry --- MUST BE LAST */ @@ -1327,8 +1327,15 @@ connectOptions2(PGconn *conn) */ if (conn->target_session_attrs) { - if (strcmp(conn->target_session_attrs, "any") != 0 - && strcmp(conn->target_session_attrs, "read-write") != 0) + if (strcmp(conn->target_session_attrs, "any") == 0) + conn->requested_session_type = SESSION_TYPE_ANY; + else if (strcmp(conn->target_session_attrs, "read-write") == 0) + conn->requested_session_type = SESSION_TYPE_READ_WRITE; + else if (strcmp(conn->target_session_attrs, "prefer-read") == 0) + conn->requested_session_type = SESSION_TYPE_PREFER_READ; + else if (strcmp(conn->target_session_attrs, "read-only") == 0) + conn->requested_session_type = SESSION_TYPE_READ_ONLY; + else { conn->status = CONNECTION_BAD; printfPQExpBuffer(&conn->errorMessage, @@ -2261,13 +2268,31 @@ keep_going: /* We will come back to here until there is if (conn->whichhost + 1 >= conn->nconnhost) { - /* - * Oops, no more hosts. An appropriate error message is already - * set up, so just set the right status. - */ - goto error_return; + if (conn->read_write_host_index >= 0) + { + /* + * Getting here means we failed to connect to read-only servers + * and should now try to connect to a read-write server again. + */ + conn->whichhost = conn->read_write_host_index; + + /* + * Reset the host index value to avoid recursion during the + * second connection attempt. + */ + conn->read_write_host_index = -2; + } + else + { + /* + * Oops, no more hosts. An appropriate error message is + * already set up, so just set the right status. + */ + goto error_return; + } } - conn->whichhost++; + else + conn->whichhost++; /* Drop any address info for previous host */ release_conn_addrinfo(conn); @@ -3474,38 +3499,139 @@ keep_going: /* We will come back to here until there is case CONNECTION_CHECK_TARGET: { /* - * If a read-write connection is required, see if we have one. + * If a read-write, prefer-read or read-only connection is + * required, see if we have one. * * Servers before 7.4 lack the transaction_read_only GUC, but * by the same token they don't have any read-only mode, so we * may just skip the test in that case. */ if (conn->sversion >= 70400 && - conn->target_session_attrs != NULL && - strcmp(conn->target_session_attrs, "read-write") == 0) + conn->requested_session_type != SESSION_TYPE_ANY) + { + if (conn->sversion < 130000) + { + /* + * 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); + return PGRES_POLLING_READING; + } + else if ((conn->transaction_read_only && + conn->requested_session_type == SESSION_TYPE_READ_WRITE) || + (!conn->transaction_read_only && + (conn->requested_session_type == SESSION_TYPE_PREFER_READ || + conn->requested_session_type == SESSION_TYPE_READ_ONLY))) + { + /* Not a requested type; fail this connection. */ + const char *displayed_host; + const char *displayed_port; + + /* + * The following scenario is possible only for the + * prefer-read mode for the next pass of the list of + * connections as it couldn't find any servers that + * are default read-only. + */ + if (conn->read_write_host_index == -2) + goto consume_checked_target_connection; + + /* 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; + + if (conn->requested_session_type == SESSION_TYPE_READ_WRITE) + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not make a writable " + "connection to server " + "\"%s:%s\"\n"), + displayed_host, displayed_port); + else + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not make a readonly " + "connection to server " + "\"%s:%s\"\n"), + displayed_host, displayed_port); + + /* Close connection politely. */ + conn->status = CONNECTION_OK; + sendTerminateConn(conn); + + /* Record read-write host index */ + if (conn->requested_session_type == SESSION_TYPE_PREFER_READ && + conn->read_write_host_index == -1) + conn->read_write_host_index = conn->whichhost; + + /* + * 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; + } + + /* obtained the requested type, consume it */ + goto consume_checked_target_connection; + } + + /* + * Requested type is prefer-read, then record this host index + * and try the other before considering it later. If requested + * type of connection is read-only, ignore this connection. + */ + if (conn->requested_session_type == SESSION_TYPE_PREFER_READ || + conn->requested_session_type == SESSION_TYPE_READ_ONLY) { /* - * 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. + * The following scenario is possible only for the + * prefer-read mode for the next pass of the list of + * connections as it couldn't find any servers that are + * default read-only. */ - if (!saveErrorMessage(conn, &savedMessage)) - goto error_return; + if (conn->read_write_host_index == -2) + goto target_accept_connection; + /* Close connection politely. */ 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); - return PGRES_POLLING_READING; + sendTerminateConn(conn); + + /* Record read-write host index */ + if (conn->requested_session_type == SESSION_TYPE_PREFER_READ && + conn->read_write_host_index == -1) + conn->read_write_host_index = conn->whichhost; + + /* + * 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; } + consume_checked_target_connection: + /* We can release the address list now. */ release_conn_addrinfo(conn); @@ -3604,11 +3730,35 @@ keep_going: /* We will come back to here until there is PQntuples(res) == 1) { char *val; + bool readonly_server; val = PQgetvalue(res, 0, 0); - if (strncmp(val, "on", 2) == 0) + readonly_server = (strncmp(val, "on", 2) == 0); + + /* + * Server is read-only and requested mode is read-write, + * ignore it. Server is read-write and requested mode is + * prefer-read, record it for the first time and try to + * consume in the next scan (it means no read-only server + * is found in the first scan). Server is read-write and + * requested mode is read-only, ignore this connection. + */ + if ((readonly_server && + conn->requested_session_type == SESSION_TYPE_READ_WRITE) || + (!readonly_server && + (conn->requested_session_type == SESSION_TYPE_PREFER_READ || + conn->requested_session_type == SESSION_TYPE_READ_ONLY))) { - /* Not writable; fail this connection. */ + /* + * The following scenario is possible only for the + * prefer-read mode for the next pass of the list of + * connections as it couldn't find any servers that + * are default read-only. + */ + if (conn->read_write_host_index == -2) + goto consume_checked_write_connection; + + /* Not a requested type; fail this connection. */ PQclear(res); restoreErrorMessage(conn, &savedMessage); @@ -3621,16 +3771,28 @@ keep_going: /* We will come back to here until there is 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); + if (conn->requested_session_type == SESSION_TYPE_READ_WRITE) + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not make a writable " + "connection to server " + "\"%s:%s\"\n"), + displayed_host, displayed_port); + else + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not make a readonly " + "connection to server " + "\"%s:%s\"\n"), + displayed_host, displayed_port); /* Close connection politely. */ conn->status = CONNECTION_OK; sendTerminateConn(conn); + /* Record read-write host index */ + if (conn->requested_session_type == SESSION_TYPE_PREFER_READ && + conn->read_write_host_index == -1) + conn->read_write_host_index = conn->whichhost; + /* * Try next host if any, but we don't want to consider * additional addresses for this host. @@ -3639,7 +3801,8 @@ keep_going: /* We will come back to here until there is goto keep_going; } - /* Session is read-write, so we're good. */ + consume_checked_write_connection: + /* Session is requested type, so we're good. */ PQclear(res); termPQExpBuffer(&savedMessage); @@ -3817,6 +3980,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; @@ -3824,6 +3988,9 @@ makeEmptyPGconn(void) conn->try_gss = true; #endif + conn->requested_session_type = SESSION_TYPE_ANY; + conn->read_write_host_index = -1; + /* * We try to send at least 8K at a time, which is the usual size of pipe * buffers on Unix systems. That way, when we are sending a large amount diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index b3c59a0..3c17100 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-fe.h b/src/interfaces/libpq/libpq-fe.h index 5f65db3..aa6f22f 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -73,6 +73,14 @@ typedef enum typedef enum { + SESSION_TYPE_ANY = 0, /* Any session (default) */ + SESSION_TYPE_READ_WRITE, /* Read-write session */ + SESSION_TYPE_PREFER_READ, /* Prefer read only session */ + SESSION_TYPE_READ_ONLY /* Read only session */ +} TargetSessionAttrsType; + +typedef enum +{ PGRES_POLLING_FAILED = 0, PGRES_POLLING_READING, /* These two indicate that one may */ PGRES_POLLING_WRITING, /* use select before polling again. */ diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 64468ab..eb687d8 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -367,8 +367,12 @@ struct pg_conn char *krbsrvname; /* Kerberos service name */ #endif - /* Type of connection to make. Possible values: any, read-write. */ + /* + * Type of connection to make. Possible values: any, read-write, + * prefer-read and read-only. + */ char *target_session_attrs; + TargetSessionAttrsType requested_session_type; /* Optional file to write trace info to */ FILE *Pfdebug; @@ -403,6 +407,14 @@ struct pg_conn pg_conn_host *connhost; /* details about each named host */ char *connip; /* IP address for current network connection */ + /* + * First read-write host index in the connection string. + * + * Initial value is -1, then the index of the first read-write host, -2 + * during the second attempt of connection to avoid recursion. + */ + int read_write_host_index; + /* Connection data */ pgsocket sock; /* FD for socket, PGINVALID_SOCKET if * unconnected */ @@ -433,6 +445,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 */ diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index 3c743d7..ac1e11e 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -3,7 +3,7 @@ use strict; use warnings; use PostgresNode; use TestLib; -use Test::More tests => 32; +use Test::More tests => 37; # Initialize master node my $node_master = get_new_node('master'); @@ -121,6 +121,26 @@ test_target_session_attrs($node_master, $node_standby_1, $node_master, "any", test_target_session_attrs($node_standby_1, $node_master, $node_standby_1, "any", 0); +# Connect to standby1 in "prefer-read" mode with master,standby1 list. +test_target_session_attrs($node_master, $node_standby_1, $node_standby_1, "prefer-read", + 0); + +# Connect to standby1 in "prefer-read" mode with standby1,master list. +test_target_session_attrs($node_standby_1, $node_master, $node_standby_1, + "prefer-read", 0); + +# Connect to node_master in "prefer-read" mode with only master list. +test_target_session_attrs($node_master, $node_master, $node_master, + "prefer-read", 0); + +# Connect to standby1 in "read-only" mode with master,standby1 list. +test_target_session_attrs($node_master, $node_standby_1, $node_standby_1, + "read-only", 0); + +# Connect to standby1 in "read-only" mode with standby1,master list. +test_target_session_attrs($node_standby_1, $node_master, $node_standby_1, + "read-only", 0); + # Test for SHOW commands using a WAL sender connection with a replication # role. note "testing SHOW commands for replication connection"; -- 1.8.3.1