From 6ce04730b7fcc6fa532cbae3a2c149e3da6ae03c Mon Sep 17 00:00:00 2001 From: Greg Nancarrow Date: Mon, 30 Sep 2019 15:45:46 +1000 Subject: [PATCH v14 3/3] Server recovery mode handling Add "in_recovery" as a GUC_REPORT variable, to update clients when the server is in recovery mode. This improves the speed of client connections to a standby server, by avoiding the need to execute a command to determine if the server is in recovery mode. Add new SIGUSR1 handling interrupt to support reporting of recovery mode exit to all backends and their respective clients. Some parts of the code is taken from earlier development by Elvis Pranskevichus and Tsunakawa Takayuki. Discussion: https://www.postgresql.org/message-id/flat/CAF3+xM+8-ztOkaV9gHiJ3wfgENTq97QcjXQt+rbFQ6F7oNzt9A@mail.gmail.com --- doc/src/sgml/libpq.sgml | 16 ++-- doc/src/sgml/protocol.sgml | 8 +- src/backend/access/transam/xlog.c | 3 + src/backend/storage/ipc/procarray.c | 28 +++++++ src/backend/storage/ipc/procsignal.c | 3 + src/backend/storage/ipc/standby.c | 9 +++ src/backend/tcop/postgres.c | 60 +++++++++++++++ src/backend/utils/init/postinit.c | 6 +- src/backend/utils/misc/check_guc | 2 +- src/backend/utils/misc/guc.c | 16 ++++ src/include/storage/procarray.h | 1 + src/include/storage/procsignal.h | 2 + src/include/storage/standby.h | 1 + src/include/tcop/tcopprot.h | 2 + src/interfaces/libpq/fe-connect.c | 142 +++++++++++++++++++++++------------ src/interfaces/libpq/fe-exec.c | 4 + src/interfaces/libpq/libpq-int.h | 1 + 17 files changed, 245 insertions(+), 59 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 5f31fd0..9f66fb8 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1732,8 +1732,12 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname To determine whether the server is in recovery mode, the query - SELECT pg_is_in_recovery() will be sent upon any successful connection; - if it returns t, it means the server is in recovery mode. + SELECT pg_is_in_recovery() will be sent upon any successful connection + if the server is prior to version 13; if it returns t, it means the server + is in recovery mode. + If the server is version 13 or greater, the recovery mode state is determined by the value of + the in_recovery configuration parameter that is reported by the server upon + successful connection. @@ -2046,15 +2050,17 @@ const char *PQparameterStatus(const PGconn *conn, const char *paramName); IntervalStyle, TimeZone, integer_datetimes, - standard_conforming_strings, and - transaction_read_only. + standard_conforming_strings, + transaction_read_only and + in_recovery. (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; - transaction_read_only was not reported by releases before 13.0.) + transaction_read_only and in_recovery + were 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 87b95bc..df3953f 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1284,15 +1284,17 @@ SELECT 1/0; IntervalStyle, TimeZone, integer_datetimes, - standard_conforming_strings, and - transaction_read_only. + standard_conforming_strings, + transaction_read_only and + in_recovery. (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; - transaction_read_only was not reported by releases before 13.0.) + transaction_read_only and in_recovery + were not reported by releases before 13.0.) Note that server_version, server_encoding and diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 6c69eb6..4fe506e 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7770,6 +7770,9 @@ StartupXLOG(void) XLogCtl->SharedRecoveryInProgress = false; SpinLockRelease(&XLogCtl->info_lck); + if (standbyState != STANDBY_DISABLED) + SendRecoveryExitSignal(); + UpdateControlFile(); LWLockRelease(ControlFileLock); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 8abcfdf..744475c 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2971,6 +2971,34 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared) } /* + * SendSignalToAllBackends --- send a signal to all backends. + */ +void +SendSignalToAllBackends(ProcSignalReason reason) +{ + ProcArrayStruct *arrayP = procArray; + int index; + pid_t pid = 0; + + LWLockAcquire(ProcArrayLock, LW_SHARED); + + for (index = 0; index < arrayP->numProcs; index++) + { + int pgprocno = arrayP->pgprocnos[index]; + volatile PGPROC *proc = &allProcs[pgprocno]; + VirtualTransactionId procvxid; + + GET_VXID_FROM_PGPROC(procvxid, *proc); + + pid = proc->pid; + if (pid != 0) + (void) SendProcSignal(pid, reason, procvxid.backendId); + } + + LWLockRelease(ProcArrayLock); +} + +/* * ProcArraySetReplicationSlotXmin * * Install limits to future computations of the xmin horizon to prevent vacuum diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c index 7605b2c..e4548dc 100644 --- a/src/backend/storage/ipc/procsignal.c +++ b/src/backend/storage/ipc/procsignal.c @@ -292,6 +292,9 @@ procsignal_sigusr1_handler(SIGNAL_ARGS) if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN)) RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN); + if (CheckProcSignal(PROCSIG_RECOVERY_EXIT)) + HandleRecoveryExitInterrupt(); + SetLatch(MyLatch); latch_sigusr1_handler(); diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index 01ddffe..b0e88ee 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -138,6 +138,15 @@ ShutdownRecoveryTransactionEnvironment(void) VirtualXactLockTableCleanup(); } +/* + * SendRecoveryExitSignal + * Signal backends that the server has exited recovery mode. + */ +void +SendRecoveryExitSignal(void) +{ + SendSignalToAllBackends(PROCSIG_RECOVERY_EXIT); +} /* * ----------------------------------------------------- diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index e8d8e6f..69ce3ec 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -167,6 +167,15 @@ static bool RecoveryConflictPending = false; static bool RecoveryConflictRetryable = true; static ProcSignalReason RecoveryConflictReason; +/* + * Inbound recovery exit are initially processed by + * HandleRecoveryExitInterrupt(), called from inside a signal handler. + * That just sets the recoveryExitInterruptPending flag and sets the process + * latch. ProcessRecoveryExitInterrupt() will then be called whenever it's + * safe to actually deal with the interrupt. + */ +volatile sig_atomic_t recoveryExitInterruptPending = false; + /* reused buffer to pass to SendRowDescriptionMessage() */ static MemoryContext row_description_context = NULL; static StringInfoData row_description_buf; @@ -195,6 +204,7 @@ static void drop_unnamed_stmt(void); static void log_disconnections(int code, Datum arg); static void enable_statement_timeout(void); static void disable_statement_timeout(void); +static void ProcessRecoveryExitInterrupt(void); /* ---------------------------------------------------------------- @@ -543,6 +553,10 @@ ProcessClientReadInterrupt(bool blocked) /* Process notify interrupts, if any */ if (notifyInterruptPending) ProcessNotifyInterrupt(); + + /* Process recovery exit interrupts that happened while reading */ + if (recoveryExitInterruptPending) + ProcessRecoveryExitInterrupt(); } else if (ProcDiePending) { @@ -2962,6 +2976,52 @@ RecoveryConflictInterrupt(ProcSignalReason reason) } /* + * HandleRecoveryExitInterrupt + * + * Signal handler portion of interrupt handling. Let the backend know + * that the server has exited the recovery mode. + */ +void +HandleRecoveryExitInterrupt(void) +{ + /* + * Note: this is called by a SIGNAL HANDLER. You must be very wary what + * you do here. + */ + + /* signal that work needs to be done */ + recoveryExitInterruptPending = true; + + /* make sure the event is processed in due course */ + SetLatch(MyLatch); +} + +/* + * ProcessRecoveryExitInterrupt + * + * This is called just after waiting for a frontend command. If a + * interrupt arrives (via HandleRecoveryExitInterrupt()) while reading, + * the read will be interrupted via the process's latch, and this routine + * will get called. +*/ +static void +ProcessRecoveryExitInterrupt(void) +{ + recoveryExitInterruptPending = false; + + SetConfigOption("in_recovery", + "off", + PGC_INTERNAL, PGC_S_OVERRIDE); + + /* + * Flush output buffer so that clients receive the ParameterStatus message + * as soon as possible. + */ + pq_flush(); +} + + +/* * ProcessInterrupts: out-of-line portion of CHECK_FOR_INTERRUPTS() macro * * If an interrupt condition is pending, and it's safe to service it, diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 29c5ec7..59fb4e9 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -649,7 +649,11 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, * This is handled by calling RecoveryInProgress and ignoring the * result. */ - (void) RecoveryInProgress(); + if (RecoveryInProgress()) + SetConfigOption("in_recovery", + "on", + PGC_INTERNAL, PGC_S_OVERRIDE); + } else { diff --git a/src/backend/utils/misc/check_guc b/src/backend/utils/misc/check_guc index 416a087..a4ebcef 100755 --- a/src/backend/utils/misc/check_guc +++ b/src/backend/utils/misc/check_guc @@ -21,7 +21,7 @@ is_superuser lc_collate lc_ctype lc_messages lc_monetary lc_numeric lc_time \ pre_auth_delay role seed server_encoding server_version server_version_num \ session_authorization trace_lock_oidmin trace_lock_table trace_locks trace_lwlocks \ trace_notify trace_userlocks transaction_isolation transaction_read_only \ -zero_damaged_pages" +zero_damaged_pages in_recovery" ### What options are listed in postgresql.conf.sample, but don't appear ### in guc.c? diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index c64ec03..633bc2d 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -580,6 +580,7 @@ static char *recovery_target_string; static char *recovery_target_xid_string; static char *recovery_target_name_string; static char *recovery_target_lsn_string; +static bool in_recovery; /* should be static, but commands/variable.c needs to get at this */ @@ -1770,6 +1771,21 @@ static struct config_bool ConfigureNamesBool[] = }, { + /* + * Not for general use --- used to indicate whether the instance is + * recovery mode + */ + {"in_recovery", PGC_INTERNAL, UNGROUPED, + gettext_noop("Shows whether the instance is in recovery mode."), + NULL, + GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE + }, + &in_recovery, + false, + NULL, NULL, NULL + }, + + { {"allow_system_table_mods", PGC_POSTMASTER, DEVELOPER_OPTIONS, gettext_noop("Allows modifications of the structure of system tables."), NULL, diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h index da8b672..86f0c13 100644 --- a/src/include/storage/procarray.h +++ b/src/include/storage/procarray.h @@ -113,6 +113,7 @@ extern void CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conf extern int CountUserBackends(Oid roleid); extern bool CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared); +extern void SendSignalToAllBackends(ProcSignalReason reason); extern void XidCacheRemoveRunningXids(TransactionId xid, int nxids, const TransactionId *xids, diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h index 05b186a..9cf9560 100644 --- a/src/include/storage/procsignal.h +++ b/src/include/storage/procsignal.h @@ -42,6 +42,8 @@ typedef enum PROCSIG_RECOVERY_CONFLICT_BUFFERPIN, PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK, + PROCSIG_RECOVERY_EXIT, /* recovery exit interrupt */ + NUM_PROCSIGNALS /* Must be last! */ } ProcSignalReason; diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h index a3f8f82..2c73f0c 100644 --- a/src/include/storage/standby.h +++ b/src/include/storage/standby.h @@ -26,6 +26,7 @@ extern int max_standby_streaming_delay; extern void InitRecoveryTransactionEnvironment(void); extern void ShutdownRecoveryTransactionEnvironment(void); +extern void SendRecoveryExitSignal(void); extern void ResolveRecoveryConflictWithSnapshot(TransactionId latestRemovedXid, RelFileNode node); diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index ec21f7e..ed21a9e 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -66,6 +66,8 @@ extern void StatementCancelHandler(SIGNAL_ARGS); extern void FloatExceptionHandler(SIGNAL_ARGS) pg_attribute_noreturn(); extern void RecoveryConflictInterrupt(ProcSignalReason reason); /* called from SIGUSR1 * handler */ +/* recovery exit interrupt handling function */ +extern void HandleRecoveryExitInterrupt(void); extern void ProcessClientReadInterrupt(bool blocked); extern void ProcessClientWriteInterrupt(bool blocked); diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 0c096d1..7a04618 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2207,6 +2207,58 @@ reject_checked_read_or_write_connection(PGconn *conn) conn->try_next_host = true; } +/* + * Internal helper function used for rejecting (and closing) a connection that + * doesn't satisfy the requested session type (for recovery). The connection state + * is set to try the next host (if any). + * In the case of SESSION_TYPE_PREFER_STANDBY, if the read-write-or-primary host-index + * hasn't been set, then it is set to the index of this connection's host, so that a + * connection to this host can be made again in the event that no connection to + * a standby host could be made after the first host scan. + */ +static void +reject_checked_recovery_connection(PGconn *conn) +{ + /* Not a requested type; 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; + + if (conn->requested_session_type == SESSION_TYPE_PRIMARY) + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("server is in recovery mode " + "\"%s:%s\"\n"), + displayed_host, displayed_port); + else + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("server is not in recovery mode " + "\"%s:%s\"\n"), + displayed_host, displayed_port); + + /* Close connection politely. */ + conn->status = CONNECTION_OK; + sendTerminateConn(conn); + + /* Record primary host index */ + if (conn->requested_session_type == SESSION_TYPE_PREFER_STANDBY && + conn->read_write_or_primary_host_index == -1) + conn->read_write_or_primary_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; +} + /* ---------------- * PQconnectPoll * @@ -3630,27 +3682,52 @@ keep_going: /* We will come back to here until there is conn->requested_session_type == SESSION_TYPE_PREFER_STANDBY || conn->requested_session_type == SESSION_TYPE_STANDBY))) { - /* - * 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, "SELECT pg_is_in_recovery()")) + 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, "SELECT pg_is_in_recovery()")) + { + restoreErrorMessage(conn, &savedMessage); + goto error_return; + } + + conn->status = CONNECTION_CHECK_RECOVERY; + restoreErrorMessage(conn, &savedMessage); - goto error_return; + return PGRES_POLLING_READING; } + else if ((conn->in_recovery && + conn->requested_session_type == SESSION_TYPE_PRIMARY) || + (!conn->in_recovery && + (conn->requested_session_type == SESSION_TYPE_PREFER_STANDBY || + conn->requested_session_type == SESSION_TYPE_STANDBY))) + { + /* + * The following scenario is possible only for the + * prefer-standby mode for the next pass of the list + * of connections as it couldn't find any servers that + * are in recovery. + */ + if (conn->read_write_or_primary_host_index == -2) + goto consume_checked_target_connection; - conn->status = CONNECTION_CHECK_RECOVERY; + reject_checked_recovery_connection(conn); + goto keep_going; + } - restoreErrorMessage(conn, &savedMessage); - return PGRES_POLLING_READING; + /* obtained the requested type, consume it */ + goto consume_checked_target_connection; } /* @@ -3932,40 +4009,7 @@ keep_going: /* We will come back to here until there is PQclear(res); restoreErrorMessage(conn, &savedMessage); - /* 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_PRIMARY) - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("server is in recovery mode " - "\"%s:%s\"\n"), - displayed_host, displayed_port); - else - appendPQExpBuffer(&conn->errorMessage, - libpq_gettext("server is not in recovery mode " - "\"%s:%s\"\n"), - displayed_host, displayed_port); - - /* Close connection politely. */ - conn->status = CONNECTION_OK; - sendTerminateConn(conn); - - /* Record primary host index */ - if (conn->requested_session_type == SESSION_TYPE_PREFER_STANDBY && - conn->read_write_or_primary_host_index == -1) - conn->read_write_or_primary_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; + reject_checked_recovery_connection(conn); goto keep_going; } diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 3c17100..a964483 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -1117,6 +1117,10 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value) { conn->transaction_read_only = (strcmp(value, "on") == 0); } + else if (strcmp(name, "in_recovery") == 0) + { + conn->in_recovery = (strcmp(value, "on") == 0); + } } diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 0efe16f..327dd04 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -446,6 +446,7 @@ struct pg_conn int client_encoding; /* encoding id */ bool std_strings; /* standard_conforming_strings */ bool transaction_read_only; /* transaction_read_only */ + bool in_recovery; /* in_recovery */ PGVerbosity verbosity; /* error/notice message verbosity */ PGContextVisibility show_context; /* whether to show CONTEXT field */ PGlobjfuncs *lobjfuncs; /* private state for large-object access fns */ -- 1.8.3.1