public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v6 3/3] Allow recovery via loadable modules. 3+ messages / 3 participants [nested] [flat]
* [PATCH v6 3/3] Allow recovery via loadable modules. @ 2022-12-10 03:40 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Nathan Bossart @ 2022-12-10 03:40 UTC (permalink / raw) This adds the restore_library parameter to allow archive recovery via a loadable module, rather than running shell commands. --- contrib/basic_archive/Makefile | 4 +- contrib/basic_archive/basic_archive.c | 67 ++++++- contrib/basic_archive/meson.build | 7 +- contrib/basic_archive/t/001_restore.pl | 44 +++++ doc/src/sgml/archive-modules.sgml | 168 ++++++++++++++++-- doc/src/sgml/backup.sgml | 43 ++++- doc/src/sgml/basic-archive.sgml | 33 ++-- doc/src/sgml/config.sgml | 54 +++++- doc/src/sgml/high-availability.sgml | 23 ++- src/backend/access/transam/shell_restore.c | 21 ++- src/backend/access/transam/xlog.c | 13 +- src/backend/access/transam/xlogarchive.c | 70 +++++++- src/backend/access/transam/xlogrecovery.c | 26 ++- src/backend/postmaster/checkpointer.c | 26 +++ src/backend/postmaster/pgarch.c | 7 +- src/backend/postmaster/startup.c | 23 ++- src/backend/utils/misc/guc.c | 14 ++ src/backend/utils/misc/guc_tables.c | 10 ++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/access/xlog_internal.h | 1 + src/include/access/xlogarchive.h | 44 ++++- src/include/access/xlogrecovery.h | 1 + src/include/utils/guc.h | 2 + 23 files changed, 618 insertions(+), 84 deletions(-) create mode 100644 contrib/basic_archive/t/001_restore.pl diff --git a/contrib/basic_archive/Makefile b/contrib/basic_archive/Makefile index 55d299d650..487dc563f3 100644 --- a/contrib/basic_archive/Makefile +++ b/contrib/basic_archive/Makefile @@ -1,7 +1,7 @@ # contrib/basic_archive/Makefile MODULES = basic_archive -PGFILEDESC = "basic_archive - basic archive module" +PGFILEDESC = "basic_archive - basic archive and recovery module" REGRESS = basic_archive REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/basic_archive/basic_archive.conf @@ -9,6 +9,8 @@ REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/basic_archive/basic_archive.c # which typical installcheck users do not have (e.g. buildfarm clients). NO_INSTALLCHECK = 1 +TAP_TESTS = 1 + ifdef USE_PGXS PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c index 28cbb6cce0..8c333c8f99 100644 --- a/contrib/basic_archive/basic_archive.c +++ b/contrib/basic_archive/basic_archive.c @@ -17,6 +17,11 @@ * a file is successfully archived and then the system crashes before * a durable record of the success has been made. * + * This file also demonstrates a basic restore library implementation that + * is roughly equivalent to the following shell command: + * + * cp /path/to/archivedir/%f %p + * * Copyright (c) 2022-2023, PostgreSQL Global Development Group * * IDENTIFICATION @@ -30,6 +35,7 @@ #include <sys/time.h> #include <unistd.h> +#include "access/xlogarchive.h" #include "common/int.h" #include "miscadmin.h" #include "postmaster/pgarch.h" @@ -48,6 +54,8 @@ static bool basic_archive_file(const char *file, const char *path); static void basic_archive_file_internal(const char *file, const char *path); static bool check_archive_directory(char **newval, void **extra, GucSource source); static bool compare_files(const char *file1, const char *file2); +static bool basic_restore_file(const char *file, const char *path, + const char *lastRestartPointFileName); /* * _PG_init @@ -87,6 +95,19 @@ _PG_archive_module_init(ArchiveModuleCallbacks *cb) cb->archive_file_cb = basic_archive_file; } +/* + * _PG_recovery_module_init + * + * Returns the module's restore callback. + */ +void +_PG_recovery_module_init(RecoveryModuleCallbacks *cb) +{ + AssertVariableIsOfType(&_PG_recovery_module_init, RecoveryModuleInit); + + cb->restore_cb = basic_restore_file; +} + /* * check_archive_directory * @@ -99,8 +120,8 @@ check_archive_directory(char **newval, void **extra, GucSource source) /* * The default value is an empty string, so we have to accept that value. - * Our check_configured callback also checks for this and prevents - * archiving from proceeding if it is still empty. + * Our check_configured and restore callbacks also check for this and + * prevent archiving or recovery from proceeding if it is still empty. */ if (*newval == NULL || *newval[0] == '\0') return true; @@ -368,3 +389,45 @@ compare_files(const char *file1, const char *file2) return ret; } + +/* + * basic_restore_file + * + * Retrieves one file from the WAL archives. + */ +static bool +basic_restore_file(const char *file, const char *path, + const char *lastRestartPointFileName) +{ + char source[MAXPGPATH]; + struct stat st; + + ereport(DEBUG1, + (errmsg("restoring \"%s\" via basic_archive", file))); + + if (archive_directory == NULL || archive_directory[0] == '\0') + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"basic_archive.archive_directory\" is not set"))); + + /* + * Check whether the file exists. If not, we return false to indicate that + * there are no more files to restore. + */ + snprintf(source, MAXPGPATH, "%s/%s", archive_directory, file); + if (stat(source, &st) != 0) + { + int elevel = (errno == ENOENT) ? DEBUG1 : ERROR; + + ereport(elevel, + (errcode_for_file_access(), + errmsg("could not stat file \"%s\": %m", source))); + return false; + } + + copy_file(source, unconstify(char *, path)); + + ereport(DEBUG1, + (errmsg("restored \"%s\" via basic_archive", file))); + return true; +} diff --git a/contrib/basic_archive/meson.build b/contrib/basic_archive/meson.build index bc1380e6f6..af4580dea9 100644 --- a/contrib/basic_archive/meson.build +++ b/contrib/basic_archive/meson.build @@ -7,7 +7,7 @@ basic_archive_sources = files( if host_system == 'windows' basic_archive_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ '--NAME', 'basic_archive', - '--FILEDESC', 'basic_archive - basic archive module',]) + '--FILEDESC', 'basic_archive - basic archive and recovery module',]) endif basic_archive = shared_module('basic_archive', @@ -31,4 +31,9 @@ tests += { # which typical runningcheck users do not have (e.g. buildfarm clients). 'runningcheck': false, }, + 'tap': { + 'tests': [ + 't/001_restore.pl', + ], + }, } diff --git a/contrib/basic_archive/t/001_restore.pl b/contrib/basic_archive/t/001_restore.pl new file mode 100644 index 0000000000..ec8767d740 --- /dev/null +++ b/contrib/basic_archive/t/001_restore.pl @@ -0,0 +1,44 @@ + +# Copyright (c) 2022, PostgreSQL Global Development Group + +use strict; +use warnings; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# start a node +my $node = PostgreSQL::Test::Cluster->new('node'); +$node->init(has_archiving => 1, allows_streaming => 1); +my $archive_dir = $node->archive_dir; +$archive_dir =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os; +$node->append_conf('postgresql.conf', "archive_command = ''"); +$node->append_conf('postgresql.conf', "archive_library = 'basic_archive'"); +$node->append_conf('postgresql.conf', "basic_archive.archive_directory = '$archive_dir'"); +$node->start; + +# backup the node +my $backup = 'backup'; +$node->backup($backup); + +# generate some new WAL files +$node->safe_psql('postgres', "CREATE TABLE test (a INT);"); +$node->safe_psql('postgres', "SELECT pg_switch_wal();"); +$node->safe_psql('postgres', "INSERT INTO test VALUES (1);"); + +# shut down the node (this should archive all WAL files) +$node->stop; + +# restore from the backup +my $restore = PostgreSQL::Test::Cluster->new('restore'); +$restore->init_from_backup($node, $backup, has_restoring => 1, standby => 0); +$restore->append_conf('postgresql.conf', "restore_command = ''"); +$restore->append_conf('postgresql.conf', "restore_library = 'basic_archive'"); +$restore->append_conf('postgresql.conf', "basic_archive.archive_directory = '$archive_dir'"); +$restore->start; + +# ensure post-backup WAL was replayed +my $result = $restore->safe_psql("postgres", "SELECT count(*) FROM test;"); +is($result, "1", "check restore content"); + +done_testing(); diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml index ef02051f7f..53e657040b 100644 --- a/doc/src/sgml/archive-modules.sgml +++ b/doc/src/sgml/archive-modules.sgml @@ -1,34 +1,40 @@ <!-- doc/src/sgml/archive-modules.sgml --> <chapter id="archive-modules"> - <title>Archive Modules</title> + <title>Archive and Recovery Modules</title> <indexterm zone="archive-modules"> - <primary>Archive Modules</primary> + <primary>Archive and Recovery Modules</primary> </indexterm> <para> PostgreSQL provides infrastructure to create custom modules for continuous - archiving (see <xref linkend="continuous-archiving"/>). While archiving via - a shell command (i.e., <xref linkend="guc-archive-command"/>) is much - simpler, a custom archive module will often be considerably more robust and - performant. + archiving and recovery (see <xref linkend="continuous-archiving"/>). While + a shell command (e.g., <xref linkend="guc-archive-command"/>, + <xref linkend="guc-restore-command"/>) is much simpler, a custom module will + often be considerably more robust and performant. </para> <para> When a custom <xref linkend="guc-archive-library"/> is configured, PostgreSQL will submit completed WAL files to the module, and the server will avoid recycling or removing these WAL files until the module indicates that the files - were successfully archived. It is ultimately up to the module to decide what - to do with each WAL file, but many recommendations are listed at - <xref linkend="backup-archiving-wal"/>. + were successfully archived. When a custom + <xref linkend="guc-restore-library"/> is configured, PostgreSQL will use the + module for recovery actions. It is ultimately up to the module to decide how + to accomplish each task, but some recommendations are listed at + <xref linkend="backup-archiving-wal"/> and + <xref linkend="backup-pitr-recovery"/>. </para> <para> - Archiving modules must at least consist of an initialization function (see - <xref linkend="archive-module-init"/>) and the required callbacks (see - <xref linkend="archive-module-callbacks"/>). However, archive modules are - also permitted to do much more (e.g., declare GUCs and register background - workers). + Archive and recovery modules must at least consist of an initialization + function (see <xref linkend="archive-module-init"/> and + <xref linkend="recovery-module-init"/>) and the required callbacks (see + <xref linkend="archive-module-callbacks"/> and + <xref linkend="recovery-module-callbacks"/>). However, archive and recovery + modules are also permitted to do much more (e.g., declare GUCs and register + background workers). A module may be used for both + <varname>archive_library</varname> and <varname>restore_library</varname>. </para> <para> @@ -37,7 +43,7 @@ </para> <sect1 id="archive-module-init"> - <title>Initialization Functions</title> + <title>Archive Module Initialization Functions</title> <indexterm zone="archive-module-init"> <primary>_PG_archive_module_init</primary> </indexterm> @@ -64,6 +70,12 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb); Only the <function>archive_file_cb</function> callback is required. The others are optional. </para> + + <note> + <para> + <varname>archive_library</varname> is only loaded in the archiver process. + </para> + </note> </sect1> <sect1 id="archive-module-callbacks"> @@ -129,6 +141,132 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path); <programlisting> typedef void (*ArchiveShutdownCB) (void); +</programlisting> + </para> + </sect2> + </sect1> + + <sect1 id="recovery-module-init"> + <title>Recovery Module Initialization Functions</title> + <indexterm zone="recovery-module-init"> + <primary>_PG_recovery_module_init</primary> + </indexterm> + <para> + A recovery library is loaded by dynamically loading a shared library with the + <xref linkend="guc-restore-library"/> as the library base name. The normal + library search path is used to locate the library. To provide the required + recovery module callbacks and to indicate that the library is actually a + recovery module, it needs to provide a function named + <function>_PG_recovery_module_init</function>. This function is passed a + struct that needs to be filled with the callback function pointers for + individual actions. + +<programlisting> +typedef struct RecoveryModuleCallbacks +{ + RecoveryRestoreCB restore_cb; + RecoveryArchiveCleanupCB archive_cleanup_cb; + RecoveryEndCB recovery_end_cb; + RecoveryShutdownCB shutdown_cb; +} RecoveryModuleCallbacks; +typedef void (*RecoveryModuleInit) (struct RecoveryModuleCallbacks *cb); +</programlisting> + + The <function>restore_cb</function> callback is required for archive + recovery, but it is optional for streaming replication. The others are + always optional. + </para> + + <note> + <para> + <varname>restore_library</varname> is only loaded in the startup and + checkpointer processes and in single-user mode. + </para> + </note> + </sect1> + + <sect1 id="recovery-module-callbacks"> + <title>Recovery Module Callbacks</title> + <para> + The recovery callbacks define the actual behavior of the module. The server + will call them as required to execute recovery actions. + </para> + + <sect2 id="recovery-module-restore"> + <title>Restore Callback</title> + <para> + The <function>restore_cb</function> callback is called to retrieve a single + archived segment of the WAL file series for archive recovery or streaming + replication. + +<programlisting> +typedef bool (*RecoveryRestoreCB) (const char *file, const char *path, const char *lastRestartPointFileName); +</programlisting> + + This callback must return <literal>true</literal> only if the file was + successfully retrieved. If the file is not available in the archives, the + callback must return <literal>false</literal>. + <replaceable>file</replaceable> will contain just the file name + of the WAL file to retrieve, while <replaceable>path</replaceable> contains + the destination's relative path (including the file name). + <replaceable>lastRestartPointFileName</replaceable> will contain the name + of the file containing the last valid restart point. That is the earliest + file that must be kept to allow a restore to be restartable, so this + information can be used to truncate the archive to just the minimum + required to support restarting from the current restore. + <replaceable>lastRestartPointFileName</replaceable> is typically only used + by warm-standby configurations (see <xref linkend="warm-standby"/>). Note + that if multiple standby servers are restoring from the same archive + directory, you will need to ensure that you do not delete WAL files until + they are no longer needed by any of the servers. + </para> + </sect2> + + <sect2 id="recovery-module-archive-cleanup"> + <title>Archive Cleanup Callback</title> + <para> + The <function>archive_cleanup_cb</function> callback is called at every + restart point and is intended to provide a mechanism for cleaning up old + archived WAL files that are no longer needed by the standby server. + +<programlisting> +typedef void (*RecoveryArchiveCleanupCB) (const char *lastRestartPointFileName); +</programlisting> + + <replaceable>lastRestartPointFileName</replaceable> will contain the name + of the file containing the last valid restart point, like in + <link linkend="recovery-module-restore"><function>restore_cb</function></link>. + </para> + </sect2> + + <sect2 id="recovery-module-end"> + <title>Recovery End Callback</title> + <para> + The <function>recovery_end_cb</function> callback is called once at the end + of recovery and is intended to provide a mechanism for cleanup following + replication or recovery. + +<programlisting> +typedef void (*RecoveryEndCB) (const char *lastRestartPointFileName); +</programlisting> + + <replaceable>lastRestartPointFileName</replaceable> will contain the name + of the file containing the last valid restart point, like in + <link linkend="recovery-module-restore"><function>restore_cb</function></link>. + </para> + </sect2> + + <sect2 id="recovery-module-shutdown"> + <title>Shutdown Callback</title> + <para> + The <function>shutdown_cb</function> callback is called when a process that + has loaded the recovery module exits (e.g., after an error) or the value of + <xref linkend="guc-restore-library"/> changes. If no + <function>shutdown_cb</function> is defined, no special action is taken in + these situations. + +<programlisting> +typedef void (*RecoveryShutdownCB) (void); </programlisting> </para> </sect2> diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index 8bab521718..d6d34078fc 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1181,9 +1181,27 @@ SELECT * FROM pg_backup_stop(wait_for_archive => true); <para> The key part of all this is to set up a recovery configuration that describes how you want to recover and how far the recovery should - run. The one thing that you absolutely must specify is the <varname>restore_command</varname>, - which tells <productname>PostgreSQL</productname> how to retrieve archived - WAL file segments. Like the <varname>archive_command</varname>, this is + run. The one thing that you absolutely must specify is either + <varname>restore_command</varname> or a <varname>restore_library</varname> + that defines a restore callback, which tells + <productname>PostgreSQL</productname> how to retrieve archived WAL file + segments. + </para> + + <para> + Like the <varname>archive_library</varname> parameter, + <varname>restore_library</varname> is a shared library. Since such + libraries are written in <literal>C</literal>, creating your own may + require considerably more effort than writing a shell command. However, + recovery modules can be more performant than restoring via shell, and they + will have access to many useful server resources. For more information + about creating a <varname>restore_library</varname>, see + <xref linkend="archive-modules"/>. + </para> + + <para> + Like the <varname>archive_command</varname>, + <varname>restore_command</varname> is a shell command string. It can contain <literal>%f</literal>, which is replaced by the name of the desired WAL file, and <literal>%p</literal>, which is replaced by the path name to copy the WAL file to. @@ -1202,14 +1220,20 @@ restore_command = 'cp /mnt/server/archivedir/%f %p' </para> <para> - It is important that the command return nonzero exit status on failure. - The command <emphasis>will</emphasis> be called requesting files that are not - present in the archive; it must return nonzero when so asked. This is not - an error condition. An exception is that if the command was terminated by + It is important that the <varname>restore_command</varname> return nonzero + exit status on failure, or, if you are using a + <varname>restore_library</varname>, that the restore function returns + <literal>false</literal> on failure. The command or library + <emphasis>will</emphasis> be called requesting files that are not + present in the archive; it must fail when so asked. This is not + an error condition. An exception is that if the + <varname>restore_command</varname> was terminated by a signal (other than <systemitem>SIGTERM</systemitem>, which is used as part of a database server shutdown) or an error by the shell (such as command not found), then recovery will abort and the server will not start - up. + up. Likewise, if the restore function provided by the + <varname>restore_library</varname> emits an <literal>ERROR</literal> or + <literal>FATAL</literal>, recovery will abort and the server won't start. </para> <para> @@ -1233,7 +1257,8 @@ restore_command = 'cp /mnt/server/archivedir/%f %p' close as possible given the available WAL segments). Therefore, a normal recovery will end with a <quote>file not found</quote> message, the exact text of the error message depending upon your choice of - <varname>restore_command</varname>. You may also see an error message + <varname>restore_command</varname> or <varname>restore_library</varname>. + You may also see an error message at the start of recovery for a file named something like <filename>00000001.history</filename>. This is also normal and does not indicate a problem in simple recovery situations; see diff --git a/doc/src/sgml/basic-archive.sgml b/doc/src/sgml/basic-archive.sgml index 60f23d2855..11fd670dbc 100644 --- a/doc/src/sgml/basic-archive.sgml +++ b/doc/src/sgml/basic-archive.sgml @@ -8,17 +8,20 @@ </indexterm> <para> - <filename>basic_archive</filename> is an example of an archive module. This - module copies completed WAL segment files to the specified directory. This - may not be especially useful, but it can serve as a starting point for - developing your own archive module. For more information about archive - modules, see <xref linkend="archive-modules"/>. + <filename>basic_archive</filename> is an example of an archive and recovery + module. This module copies completed WAL segment files to or from the + specified directory. This may not be especially useful, but it can serve as + a starting point for developing your own archive and recovery modules. For + more information about archive and recovery modules, see + see <xref linkend="archive-modules"/>. </para> <para> - In order to function, this module must be loaded via + For use as an archive module, this module must be loaded via <xref linkend="guc-archive-library"/>, and <xref linkend="guc-archive-mode"/> - must be enabled. + must be enabled. For use as a recovery module, this module must be loaded + via <xref linkend="guc-restore-library"/>, and recovery must be enabled (see + <xref linkend="runtime-config-wal-archive-recovery"/>). </para> <sect2 id="basic-archive-configuration-parameters"> @@ -34,11 +37,12 @@ </term> <listitem> <para> - The directory where the server should copy WAL segment files. This - directory must already exist. The default is an empty string, which - effectively halts WAL archiving, but if <xref linkend="guc-archive-mode"/> - is enabled, the server will accumulate WAL segment files in the - expectation that a value will soon be provided. + The directory where the server should copy WAL segment files to or from. + This directory must already exist. The default is an empty string, + which, when used for archiving, effectively halts WAL archival, but if + <xref linkend="guc-archive-mode"/> is enabled, the server will accumulate + WAL segment files in the expectation that a value will soon be provided. + When an empty string is used for recovery, restore will fail. </para> </listitem> </varlistentry> @@ -46,7 +50,7 @@ <para> These parameters must be set in <filename>postgresql.conf</filename>. - Typical usage might be: + Typical usage as an archive module might be: </para> <programlisting> @@ -61,7 +65,8 @@ basic_archive.archive_directory = '/path/to/archive/directory' <title>Notes</title> <para> - Server crashes may leave temporary files with the prefix + When <filename>basic_archive</filename> is used as an archive module, server + crashes may leave temporary files with the prefix <filename>archtemp</filename> in the archive directory. It is recommended to delete such files before restarting the server after a crash. It is safe to remove such files while the server is running as long as they are unrelated diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 77574e2d4e..039d3360ac 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3773,7 +3773,8 @@ include_dir 'conf.d' recovery when the end of archived WAL is reached, but will keep trying to continue recovery by connecting to the sending server as specified by the <varname>primary_conninfo</varname> setting and/or by fetching new WAL - segments using <varname>restore_command</varname>. For this mode, the + segments using <varname>restore_command</varname> or + <varname>restore_library</varname>. For this mode, the parameters from this section and <xref linkend="runtime-config-replication-standby"/> are of interest. Parameters from <xref linkend="runtime-config-wal-recovery-target"/> will @@ -3801,7 +3802,8 @@ include_dir 'conf.d' <listitem> <para> The local shell command to execute to retrieve an archived segment of - the WAL file series. This parameter is required for archive recovery, + the WAL file series. Either <varname>restore_command</varname> or + <xref linkend="guc-restore-library"/> is required for archive recovery, but optional for streaming replication. Any <literal>%f</literal> in the string is replaced by the name of the file to retrieve from the archive, @@ -3836,7 +3838,42 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows <para> This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. + file or on the server command line. It is only used if + <varname>restore_library</varname> is set to an empty string. If both + <varname>restore_command</varname> and + <varname>restore_library</varname> are set, an error will be raised. + </para> + </listitem> + </varlistentry> + + <varlistentry id="guc-restore-library" xreflabel="restore_library"> + <term><varname>restore_library</varname> (<type>string</type>) + <indexterm> + <primary><varname>restore_library</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + The library to use for recovery actions, including retrieving archived + segments of the WAL file series and executing tasks at restartpoints + and at recovery end. Either <xref linkend="guc-restore-command"/> or + <varname>restore_library</varname> is required for archive recovery, + but optional for streaming replication. If this parameter is set to an + empty string (the default), restoring via shell is enabled, and + <varname>restore_command</varname>, + <varname>archive_cleanup_command</varname> and + <varname>recovery_end_command</varname> are used. If both + <varname>restore_library</varname> and any of + <varname>restore_command</varname>, + <varname>archive_cleanup_command</varname> or + <varname>recovery_end_command</varname> are set, an error will be + raised. Otherwise, the specified shared library is used for recovery. + For more information, see <xref linkend="archive-modules"/>. + </para> + + <para> + This parameter can only be set in the + <filename>postgresql.conf</filename> file or on the server command line. </para> </listitem> </varlistentry> @@ -3881,7 +3918,10 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows </para> <para> This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. + file or on the server command line. It is only used if + <varname>restore_library</varname> is set to an empty string. If both + <varname>archive_cleanup_command</varname> and + <varname>restore_library</varname> are set, an error will be raised. </para> </listitem> </varlistentry> @@ -3910,11 +3950,13 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows </para> <para> This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. + file or on the server command line. It is only used if + <varname>restore_library</varname> is set to an empty string. If both + <varname>recovery_end_command</varname> and + <varname>restore_library</varname> are set, an error will be raised. </para> </listitem> </varlistentry> - </variablelist> </sect2> diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index f180607528..6266e2df7f 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -627,7 +627,8 @@ protocol to make nodes agree on a serializable transactional order. <para> In standby mode, the server continuously applies WAL received from the primary server. The standby server can read WAL from a WAL archive - (see <xref linkend="guc-restore-command"/>) or directly from the primary + (see <xref linkend="guc-restore-command"/> and + <xref linkend="guc-restore-library"/>) or directly from the primary over a TCP connection (streaming replication). The standby server will also attempt to restore any WAL found in the standby cluster's <filename>pg_wal</filename> directory. That typically happens after a server @@ -638,9 +639,11 @@ protocol to make nodes agree on a serializable transactional order. <para> At startup, the standby begins by restoring all WAL available in the - archive location, calling <varname>restore_command</varname>. Once it - reaches the end of WAL available there and <varname>restore_command</varname> - fails, it tries to restore any WAL available in the <filename>pg_wal</filename> directory. + archive location, either by calling <varname>restore_command</varname> or + by executing the <varname>restore_library</varname>'s restore callback. + Once it reaches the end of WAL available there and + <varname>restore_command</varname> or the restore callback fails, it tries + to restore any WAL available in the <filename>pg_wal</filename> directory. If that fails, and streaming replication has been configured, the standby tries to connect to the primary server and start streaming WAL from the last valid record found in archive or <filename>pg_wal</filename>. If that fails @@ -698,7 +701,8 @@ protocol to make nodes agree on a serializable transactional order. server (see <xref linkend="backup-pitr-recovery"/>). Create a file <link linkend="file-standby-signal"><filename>standby.signal</filename></link><indexterm><primary>standby.signal</primary></indexterm> in the standby's cluster data - directory. Set <xref linkend="guc-restore-command"/> to a simple command to copy files from + directory. Set <xref linkend="guc-restore-command"/> or + <xref linkend="guc-restore-library"/> to copy files from the WAL archive. If you plan to have multiple standby servers for high availability purposes, make sure that <varname>recovery_target_timeline</varname> is set to <literal>latest</literal> (the default), to make the standby server follow the timeline change @@ -707,7 +711,8 @@ protocol to make nodes agree on a serializable transactional order. <note> <para> - <xref linkend="guc-restore-command"/> should return immediately + <xref linkend="guc-restore-command"/> and restore callbacks provided by + <xref linkend="guc-restore-library"/> should return immediately if the file does not exist; the server will retry the command again if necessary. </para> @@ -731,8 +736,10 @@ protocol to make nodes agree on a serializable transactional order. <para> If you're using a WAL archive, its size can be minimized using the <xref - linkend="guc-archive-cleanup-command"/> parameter to remove files that are no - longer required by the standby server. + linkend="guc-archive-cleanup-command"/> parameter or the + <xref linkend="guc-restore-library"/>'s + <function>archive_cleanup_cb</function> callback function to remove files + that are no longer required by the standby server. The <application>pg_archivecleanup</application> utility is designed specifically to be used with <varname>archive_cleanup_command</varname> in typical single-standby configurations, see <xref linkend="pgarchivecleanup"/>. diff --git a/src/backend/access/transam/shell_restore.c b/src/backend/access/transam/shell_restore.c index 68bf6e8556..eb101b51fb 100644 --- a/src/backend/access/transam/shell_restore.c +++ b/src/backend/access/transam/shell_restore.c @@ -3,7 +3,8 @@ * shell_restore.c * * These recovery functions use a user-specified shell command (e.g., the - * restore_command GUC). + * restore_command GUC). It is used as the default, but other modules may + * define their own recovery logic. * * Copyright (c) 2022, PostgreSQL Global Development Group * @@ -23,11 +24,25 @@ #include "storage/ipc.h" #include "utils/wait_event.h" +static bool shell_restore(const char *file, const char *path, + const char *lastRestartPointFileName); +static void shell_archive_cleanup(const char *lastRestartPointFileName); +static void shell_recovery_end(const char *lastRestartPointFileName); static bool ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOnSignal, bool exitOnSigterm, uint32 wait_event_info, int fail_elevel); +void +shell_restore_init(RecoveryModuleCallbacks *cb) +{ + AssertVariableIsOfType(&shell_restore_init, RecoveryModuleInit); + + cb->restore_cb = shell_restore; + cb->archive_cleanup_cb = shell_archive_cleanup; + cb->recovery_end_cb = shell_recovery_end; +} + bool shell_restore(const char *file, const char *path, const char *lastRestartPointFileName) @@ -70,7 +85,7 @@ shell_restore(const char *file, const char *path, return ret; } -void +static void shell_archive_cleanup(const char *lastRestartPointFileName) { char *cmd; @@ -83,7 +98,7 @@ shell_archive_cleanup(const char *lastRestartPointFileName) pfree(cmd); } -void +static void shell_recovery_end(const char *lastRestartPointFileName) { char *cmd; diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 8f47fb7570..ae537cd87f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -4884,15 +4884,16 @@ static void CleanupAfterArchiveRecovery(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog, TimeLineID newTLI) { + /* - * Execute the recovery_end_command, if any. + * Execute the recovery-end callback, if any. */ - if (recoveryEndCommand && strcmp(recoveryEndCommand, "") != 0) + if (RecoveryContext.recovery_end_cb) { char lastRestartPointFname[MAXFNAMELEN]; GetOldestRestartPointFileName(lastRestartPointFname); - shell_recovery_end(lastRestartPointFname); + RecoveryContext.recovery_end_cb(lastRestartPointFname); } /* @@ -7307,14 +7308,14 @@ CreateRestartPoint(int flags) timestamptz_to_str(xtime)) : 0)); /* - * Finally, execute archive_cleanup_command, if any. + * Execute the archive-cleanup callback, if any. */ - if (archiveCleanupCommand && strcmp(archiveCleanupCommand, "") != 0) + if (RecoveryContext.archive_cleanup_cb) { char lastRestartPointFname[MAXFNAMELEN]; GetOldestRestartPointFileName(lastRestartPointFname); - shell_archive_cleanup(lastRestartPointFname); + RecoveryContext.archive_cleanup_cb(lastRestartPointFname); } return true; diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index b5cb060d55..fdab7dad43 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -22,7 +22,9 @@ #include "access/xlog.h" #include "access/xlog_internal.h" #include "access/xlogarchive.h" +#include "access/xlogrecovery.h" #include "common/archive.h" +#include "fmgr.h" #include "miscadmin.h" #include "pgstat.h" #include "postmaster/startup.h" @@ -32,6 +34,11 @@ #include "storage/ipc.h" #include "storage/lwlock.h" +/* + * Global context for recovery-related callbacks. + */ +RecoveryModuleCallbacks RecoveryContext; + /* * Attempt to retrieve the specified file from off-line archival storage. * If successful, fill "path" with its complete path (note that this will be @@ -71,7 +78,7 @@ RestoreArchivedFile(char *path, const char *xlogfname, goto not_available; /* In standby mode, restore_command might not be supplied */ - if (recoveryRestoreCommand == NULL || strcmp(recoveryRestoreCommand, "") == 0) + if (RecoveryContext.restore_cb == NULL) goto not_available; /* @@ -149,14 +156,15 @@ RestoreArchivedFile(char *path, const char *xlogfname, XLogFileName(lastRestartPointFname, 0, 0L, wal_segment_size); /* - * Check signals before restore command and reset afterwards. + * Check signals before restore callback and reset afterwards. */ PreRestoreCommand(); /* * Copy xlog from archival storage to XLOGDIR */ - ret = shell_restore(xlogfname, xlogpath, lastRestartPointFname); + ret = RecoveryContext.restore_cb(xlogfname, xlogpath, + lastRestartPointFname); PostRestoreCommand(); @@ -603,3 +611,59 @@ XLogArchiveCleanup(const char *xlog) unlink(archiveStatusPath); /* should we complain about failure? */ } + +/* + * Loads all the recovery callbacks into our global RecoveryContext. The + * caller is responsible for validating the combination of library/command + * parameters that are set (e.g., restore_command and restore_library cannot + * both be set). + */ +void +LoadRecoveryCallbacks(void) +{ + RecoveryModuleInit init; + + /* + * If the shell command is enabled, use our special initialization + * function. Otherwise, load the library and call its + * _PG_recovery_module_init(). + */ + if (restoreLibrary[0] == '\0') + init = shell_restore_init; + else + init = (RecoveryModuleInit) + load_external_function(restoreLibrary, "_PG_recovery_module_init", + false, NULL); + + if (init == NULL) + ereport(ERROR, + (errmsg("recovery modules have to define the symbol " + "_PG_recovery_module_init"))); + + memset(&RecoveryContext, 0, sizeof(RecoveryModuleCallbacks)); + (*init) (&RecoveryContext); + + /* + * If using shell commands, remove callbacks for any commands that are not + * set. + */ + if (restoreLibrary[0] == '\0') + { + if (recoveryRestoreCommand[0] == '\0') + RecoveryContext.restore_cb = NULL; + if (archiveCleanupCommand[0] == '\0') + RecoveryContext.archive_cleanup_cb = NULL; + if (recoveryEndCommand[0] == '\0') + RecoveryContext.recovery_end_cb = NULL; + } +} + +/* + * Call the shutdown callback of the loaded recovery module, if defined. + */ +void +call_recovery_module_shutdown_cb(int code, Datum arg) +{ + if (RecoveryContext.shutdown_cb) + RecoveryContext.shutdown_cb(); +} diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 5e65785306..db0cd4469a 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -80,6 +80,7 @@ const struct config_enum_entry recovery_target_action_options[] = { /* options formerly taken from recovery.conf for archive recovery */ char *recoveryRestoreCommand = NULL; +char *restoreLibrary = NULL; char *recoveryEndCommand = NULL; char *archiveCleanupCommand = NULL; RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET; @@ -1053,24 +1054,37 @@ validateRecoveryParameters(void) if (!ArchiveRecoveryRequested) return; + /* + * Check for invalid combinations of the command/library parameters and + * load the callbacks. + */ + CheckMutuallyExclusiveGUCs(restoreLibrary, "restore_library", + recoveryRestoreCommand, "restore_command"); + CheckMutuallyExclusiveGUCs(restoreLibrary, "restore_library", + recoveryEndCommand, "recovery_end_command"); + before_shmem_exit(call_recovery_module_shutdown_cb, 0); + LoadRecoveryCallbacks(); + /* * Check for compulsory parameters */ if (StandbyModeRequested) { if ((PrimaryConnInfo == NULL || strcmp(PrimaryConnInfo, "") == 0) && - (recoveryRestoreCommand == NULL || strcmp(recoveryRestoreCommand, "") == 0)) + RecoveryContext.restore_cb == NULL) ereport(WARNING, - (errmsg("specified neither primary_conninfo nor restore_command"), - errhint("The database server will regularly poll the pg_wal subdirectory to check for files placed there."))); + (errmsg("specified neither primary_conninfo nor restore_command " + "nor a restore_library that defines a restore callback"), + errhint("The database server will regularly poll the pg_wal " + "subdirectory to check for files placed there."))); } else { - if (recoveryRestoreCommand == NULL || - strcmp(recoveryRestoreCommand, "") == 0) + if (RecoveryContext.restore_cb == NULL) ereport(FATAL, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("must specify restore_command when standby mode is not enabled"))); + errmsg("must specify restore_command or a restore_library that defines " + "a restore callback when standby mode is not enabled"))); } /* diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index de0bbbfa79..6350fd0b83 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -38,6 +38,7 @@ #include "access/xlog.h" #include "access/xlog_internal.h" +#include "access/xlogarchive.h" #include "access/xlogrecovery.h" #include "libpq/pqsignal.h" #include "miscadmin.h" @@ -222,6 +223,16 @@ CheckpointerMain(void) */ before_shmem_exit(pgstat_before_server_shutdown, 0); + /* + * Check for invalid combinations of the command/library parameters and + * load the callbacks. We do this before setting up the exception handler + * so that any problems result in a server crash shortly after startup. + */ + CheckMutuallyExclusiveGUCs(restoreLibrary, "restore_library", + archiveCleanupCommand, "archive_cleanup_command"); + before_shmem_exit(call_recovery_module_shutdown_cb, 0); + LoadRecoveryCallbacks(); + /* * Create a memory context that we will do all our work in. We do this so * that we can reset the context during error recovery and thereby avoid @@ -548,6 +559,9 @@ HandleCheckpointerInterrupts(void) if (ConfigReloadPending) { + char *prevRestoreLibrary = pstrdup(restoreLibrary); + char *prevArchiveCleanupCommand = pstrdup(archiveCleanupCommand); + ConfigReloadPending = false; ProcessConfigFile(PGC_SIGHUP); @@ -563,6 +577,18 @@ HandleCheckpointerInterrupts(void) * because of SIGHUP. */ UpdateSharedMemoryConfig(); + + CheckMutuallyExclusiveGUCs(restoreLibrary, "restore_library", + archiveCleanupCommand, "archive_cleanup_command"); + if (strcmp(prevRestoreLibrary, restoreLibrary) != 0 || + strcmp(prevArchiveCleanupCommand, archiveCleanupCommand) != 0) + { + call_recovery_module_shutdown_cb(0, (Datum) 0); + LoadRecoveryCallbacks(); + } + + pfree(prevRestoreLibrary); + pfree(prevArchiveCleanupCommand); } if (ShutdownRequestPending) { diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 8ecdb9ca23..8e91f2d70f 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -831,11 +831,8 @@ LoadArchiveLibrary(void) { ArchiveModuleInit archive_init; - if (XLogArchiveLibrary[0] != '\0' && XLogArchiveCommand[0] != '\0') - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("both archive_command and archive_library set"), - errdetail("Only one of archive_command, archive_library may be set."))); + CheckMutuallyExclusiveGUCs(XLogArchiveLibrary, "archive_library", + XLogArchiveCommand, "archive_command"); memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks)); diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c index 8786186898..f9ff2b5583 100644 --- a/src/backend/postmaster/startup.c +++ b/src/backend/postmaster/startup.c @@ -20,6 +20,7 @@ #include "postgres.h" #include "access/xlog.h" +#include "access/xlogarchive.h" #include "access/xlogrecovery.h" #include "access/xlogutils.h" #include "libpq/pqsignal.h" @@ -133,13 +134,17 @@ StartupProcShutdownHandler(SIGNAL_ARGS) * Re-read the config file. * * If one of the critical walreceiver options has changed, flag xlog.c - * to restart it. + * to restart it. Also, check for invalid combinations of the command/library + * parameters and reload the recovery callbacks if necessary. */ static void StartupRereadConfig(void) { char *conninfo = pstrdup(PrimaryConnInfo); char *slotname = pstrdup(PrimarySlotName); + char *prevRestoreLibrary = pstrdup(restoreLibrary); + char *prevRestoreCommand = pstrdup(recoveryRestoreCommand); + char *prevRecoveryEndCommand = pstrdup(recoveryEndCommand); bool tempSlot = wal_receiver_create_temp_slot; bool conninfoChanged; bool slotnameChanged; @@ -161,6 +166,22 @@ StartupRereadConfig(void) if (conninfoChanged || slotnameChanged || tempSlotChanged) StartupRequestWalReceiverRestart(); + + CheckMutuallyExclusiveGUCs(restoreLibrary, "restore_library", + recoveryRestoreCommand, "restore_command"); + CheckMutuallyExclusiveGUCs(restoreLibrary, "restore_library", + recoveryEndCommand, "recovery_end_command"); + if (strcmp(prevRestoreLibrary, restoreLibrary) != 0 || + strcmp(prevRestoreCommand, recoveryRestoreCommand) != 0 || + strcmp(prevRecoveryEndCommand, recoveryEndCommand) != 0) + { + call_recovery_module_shutdown_cb(0, (Datum) 0); + LoadRecoveryCallbacks(); + } + + pfree(prevRestoreLibrary); + pfree(prevRestoreCommand); + pfree(prevRecoveryEndCommand); } /* Handle various signals that might be sent to the startup process */ diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index d52069f446..7858e9a649 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -6880,3 +6880,17 @@ call_enum_check_hook(struct config_enum *conf, int *newval, void **extra, return true; } + +/* + * ERROR if both parameters are set. + */ +void +CheckMutuallyExclusiveGUCs(const char *p1val, const char *p1name, + const char *p2val, const char *p2name) +{ + if (p1val[0] != '\0' && p2val[0] != '\0') + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("both %s and %s set", p1name, p2name), + errdetail("Only one of %s, %s may be set.", p1name, p2name))); +} diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 5025e80f89..a8a516b0c6 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -3776,6 +3776,16 @@ struct config_string ConfigureNamesString[] = NULL, NULL, NULL }, + { + {"restore_library", PGC_SIGHUP, WAL_ARCHIVE_RECOVERY, + gettext_noop("Sets the library that will be called for recovery actions."), + NULL + }, + &restoreLibrary, + "", + NULL, NULL, NULL + }, + { {"archive_cleanup_command", PGC_SIGHUP, WAL_ARCHIVE_RECOVERY, gettext_noop("Sets the shell command that will be executed at every restart point."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 4cceda4162..38fb3e0823 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -269,6 +269,7 @@ # placeholders: %p = path of file to restore # %f = file name only # e.g. 'cp /mnt/server/archivedir/%f %p' +#restore_library = '' # library to use for recovery actions #archive_cleanup_command = '' # command to execute at every restartpoint #recovery_end_command = '' # command to execute at completion of recovery diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h index 59fc7bc105..756f0898b5 100644 --- a/src/include/access/xlog_internal.h +++ b/src/include/access/xlog_internal.h @@ -400,5 +400,6 @@ extern PGDLLIMPORT bool ArchiveRecoveryRequested; extern PGDLLIMPORT bool InArchiveRecovery; extern PGDLLIMPORT bool StandbyMode; extern PGDLLIMPORT char *recoveryRestoreCommand; +extern PGDLLIMPORT char *restoreLibrary; #endif /* XLOG_INTERNAL_H */ diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h index 299304703e..71c9b88165 100644 --- a/src/include/access/xlogarchive.h +++ b/src/include/access/xlogarchive.h @@ -30,9 +30,45 @@ extern bool XLogArchiveIsReady(const char *xlog); extern bool XLogArchiveIsReadyOrDone(const char *xlog); extern void XLogArchiveCleanup(const char *xlog); -extern bool shell_restore(const char *file, const char *path, - const char *lastRestartPointFileName); -extern void shell_archive_cleanup(const char *lastRestartPointFileName); -extern void shell_recovery_end(const char *lastRestartPointFileName); +/* + * Recovery module callbacks + * + * These callback functions should be defined by recovery libraries and + * returned via _PG_recovery_module_init(). For more information about the + * purpose of each callback, refer to the recovery modules documentation. + */ +typedef bool (*RecoveryRestoreCB) (const char *file, const char *path, + const char *lastRestartPointFileName); +typedef void (*RecoveryArchiveCleanupCB) (const char *lastRestartPointFileName); +typedef void (*RecoveryEndCB) (const char *lastRestartPointFileName); +typedef void (*RecoveryShutdownCB) (void); + +typedef struct RecoveryModuleCallbacks +{ + RecoveryRestoreCB restore_cb; + RecoveryArchiveCleanupCB archive_cleanup_cb; + RecoveryEndCB recovery_end_cb; + RecoveryShutdownCB shutdown_cb; +} RecoveryModuleCallbacks; + +extern RecoveryModuleCallbacks RecoveryContext; + +/* + * Type of the shared library symbol _PG_recovery_module_init that is looked up + * when loading a recovery library. + */ +typedef void (*RecoveryModuleInit) (RecoveryModuleCallbacks *cb); + +extern PGDLLEXPORT void _PG_recovery_module_init(RecoveryModuleCallbacks *cb); + +extern void LoadRecoveryCallbacks(void); +extern void call_recovery_module_shutdown_cb(int code, Datum arg); + +/* + * Since the logic for recovery via a shell command is in the core server and + * does not need to be loaded via a shared library, it has a special + * initialization function. + */ +extern void shell_restore_init(RecoveryModuleCallbacks *cb); #endif /* XLOG_ARCHIVE_H */ diff --git a/src/include/access/xlogrecovery.h b/src/include/access/xlogrecovery.h index 47c29350f5..35d1d09374 100644 --- a/src/include/access/xlogrecovery.h +++ b/src/include/access/xlogrecovery.h @@ -55,6 +55,7 @@ extern PGDLLIMPORT int recovery_min_apply_delay; extern PGDLLIMPORT char *PrimaryConnInfo; extern PGDLLIMPORT char *PrimarySlotName; extern PGDLLIMPORT char *recoveryRestoreCommand; +extern PGDLLIMPORT char *restoreLibrary; extern PGDLLIMPORT char *recoveryEndCommand; extern PGDLLIMPORT char *archiveCleanupCommand; diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index ba89d013e6..947597247f 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -404,6 +404,8 @@ extern void *guc_malloc(int elevel, size_t size); extern pg_nodiscard void *guc_realloc(int elevel, void *old, size_t size); extern char *guc_strdup(int elevel, const char *src); extern void guc_free(void *ptr); +extern void CheckMutuallyExclusiveGUCs(const char *p1val, const char *p1name, + const char *p2val, const char *p2name); #ifdef EXEC_BACKEND extern void write_nondefault_variables(GucContext context); -- 2.25.1 --pWyiEgJYm5f9v55/-- ^ permalink raw reply [nested|flat] 3+ messages in thread
* RE: [Proposal] Add foreign-server health checks infrastructure @ 2023-03-06 11:56 Hayato Kuroda (Fujitsu) <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Hayato Kuroda (Fujitsu) @ 2023-03-06 11:56 UTC (permalink / raw) To: 'Katsuragi Yuta' <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected] <[email protected]>; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected] <[email protected]>; Kyotaro Horiguchi <[email protected]> Dear Katsuragi-san, Thank you for reviewing! PSA new version. > >> 4. the code of pqSocketPoll > >> +#if defined(POLLRDHUP) > >> + if (forConnCheck) > >> + input_fd.events |= POLLRDHUP; > >> +#endif > >> > >> I think it is better to use PQconnCheckable() to remove the macro. > > > > IIUC the macro is needed. In FreeBSD, macOS and other platforms do not > > have the > > macro POLLRDHUP so they cannot compile. I checked by my CI and got > > following error. > > > > ``` > > ... > > FAILED: src/interfaces/libpq/libpq.a.p/fe-misc.c.o > > ... > > ../src/interfaces/libpq/fe-misc.c:1149:22: error: use of undeclared > > identifier 'POLLRDHUP' > > input_fd.events |= POLLRDHUP; > > ```` > > > > It must be invisible from them. > > Sorry, my mistake... No issues :-). > >> 9. the document of postgres_fdw > >> The document of postgres_fdw_verify_connection_states_* is a little > >> bit old. Could you update it? > > > > Updated. IIUC postgres_fdw_verify_connection_states returns > > > > * true, if the connection is verified. > > * false, if the connection seems to be disconnected. > > * NULL, if this is not the supported platform or connection has not > > been established. > > > > And postgres_fdw_verify_connection_states_all returns > > > > * true if all the connections are verified. > > * false, if one of connections seems to be disconnected. > > * NULL, if this is not the supported platform or this backend has > > never established connections > > I think 'backend has never established connections' is a little bit > strong. > I think the following cases also return NULL. The case where a > connection was established, however the connection is now closed > by the postgres_fdw_disconnect() or something. NULL is also returned if > the connection is invalidated. So, I think 'NULL, if no valid > connection is established or the function is not supported on > the platform.' is better. What do you think? Disconnect functions have never been in my mind. Descriptions must be updated. > Followings are my comments for v34. Please check. > > 0001: > 1. the document of pqConnCheck > + <literal>0</literal> if the socket is valid, and returns > <literal>-1</literal> > + if the connection has already been closed or an error has > occurred. > > 1.1 if the socket is valid -> returns 0 if the 'connection is not > closed'? Fixed. > 1.2 returns -1 if the connection has already been closed <- Let me ask > a question. > Isn't this a situation where we would like to check using this > function? Is 'error has occurred' insufficient? Seems right, fixed. > 2. the comment of pqSocketCheck > + * means that the socket has not matched POLLRDHUP event and the socket > has > + * still survived. > > socket has still survived -> connection is not closed by the socket > peer? Fixed. > 3. the comment of pqSocketPoll > + * returned and forConnCheck is requested, it means that the socket has > not > + * matched POLLRDHUP event and the socket has still survived. > > socket has still survived -> connection is not closed by the socket > peer? Fixed. > 0002: > 4. the comment of verify_cached_connections > + * This function emits warnings if a disconnection is found. This > return true > + * if disconnections cannot be found, otherwise return false. > > return ture -> return's' true > return false -> return's' false Fixed. > 5. the comment of postgres_fdw_verify_connection_states > + * if the local session seems to be disconnected from other servers. > NULL is > + * returned if a connection to the specified foreign server has not > been > + * established yet, or this function is not available on this platform. > > Considering the above discussion, 'NULL is returned if a valid > connection to the specified foreign server is not established or > this function...' seems better. What do you think? Right, fixed. > 6. the document of postgres_fdw_verify_connection_states > <literal>NULL</literal> > + is returned if a connection to the specified foreign server has > not been > + established yet, or this function is not available on this > platform > > The same as comment no.5. Right, fixed. > 7. the document of postgres_fdw_verify_connection_states_all > <literal>NULL</literal> > + is returned if the local session does not have connection caches, > or this > + function is not available on this platform. > > I think there is a case where a connection cache exists but valid > connections do not exist and NULL is returned (disconnection case). > Almost the same document as the postgres_fdw_verify_connection_states > case (comment no.5) seems better. Yes, but completely same statement cannot be used because these is not specified foreign server. How about: NULL is returned if there are no established connections or this function ... > 8. the document of postgres_fdw_can_verify_connection_states > + This function checks whether > <function>postgres_fdw_verify_connection_states</function> > + and <function>postgres_fdw_verify_connection_states</function> > work well > > Should the latter (or former) postgres_fdw_verify_connection_states be > postgres_fdw_verify_connection_states_all? That was copy-and-paste error, fixed. > regards, > > -- > Katsuragi Yuta > Advanced Computing Technology Center > Research and Development Headquarters > NTT DATA CORPORATION Best Regards, Hayato Kuroda FUJITSU LIMITED Attachments: [application/octet-stream] v35-0001-Add-PQconnCheck-and-PQconnCheckable-to-libpq.patch (8.7K, ../../TYCPR01MB58700498838127C54BC9BD48F5B69@TYCPR01MB5870.jpnprd01.prod.outlook.com/2-v35-0001-Add-PQconnCheck-and-PQconnCheckable-to-libpq.patch) download | inline diff: From 7183d0caec360464dbc0e5e83c31eef837d77477 Mon Sep 17 00:00:00 2001 From: Hayato Kuroda <[email protected]> Date: Fri, 27 Jan 2023 03:17:18 +0000 Subject: [PATCH v35 1/3] Add PQconnCheck and PQconnCheckable to libpq PQconnCheck() function allows to check the status of the connection by polling the socket. This function is currently available only on systems that support the non-standard POLLRDHUP extension to the poll system call, including Linux. PQconnCheckable() checks whether the above function is available or not. --- doc/src/sgml/libpq.sgml | 38 +++++++++++++++ src/interfaces/libpq/exports.txt | 2 + src/interfaces/libpq/fe-misc.c | 79 ++++++++++++++++++++++++++------ src/interfaces/libpq/libpq-fe.h | 4 ++ 4 files changed, 108 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 3ccd8ff942..b1a4471403 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2679,6 +2679,44 @@ void *PQgetssl(const PGconn *conn); </listitem> </varlistentry> + <varlistentry id="libpq-PQconnCheck"> + <term><function>PQconnCheck</function><indexterm><primary>PQconnCheck</primary></indexterm></term> + <listitem> + <para> + Returns the health of the connection. + +<synopsis> +int PQconnCheck(PGconn *conn); +</synopsis> + </para> + + <para> + This function checks the health of the connection. Unlike <xref linkend="libpq-PQstatus"/>, + this check is performed by polling the corresponding socket. This + function is currently available only on systems that support the + non-standard <symbol>POLLRDHUP</symbol> extension to the <symbol>poll</symbol> + system call, including Linux. <xref linkend="libpq-PQconnCheck"/> + returns greater than zero if the remote peer seems to be closed, returns + <literal>0</literal> if the connection is not closed, and returns + <literal>-1</literal> if an error has occurred. + </para> + </listitem> + </varlistentry> + + <varlistentry id="libpq-PQconnCheckable"> + <term><function>PQconnCheckable</function><indexterm><primary>PQconnCheckable</primary></indexterm></term> + <listitem> + <para> + Returns true (1) or false (0) to indicate if the <xref linkend="libpq-PQconnCheck"/> + function is supported on this platform. + +<synopsis> +int PQconnCheckable(void); +</synopsis> + </para> + </listitem> + </varlistentry> + </variablelist> </para> diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index e8bcc88370..e7f0d435bd 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -186,3 +186,5 @@ PQpipelineStatus 183 PQsetTraceFlags 184 PQmblenBounded 185 PQsendFlushRequest 186 +PQconnCheck 187 +PQconnCheckable 188 diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 3653a1a8a6..897eb413b1 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -53,9 +53,10 @@ static int pqPutMsgBytes(const void *buf, size_t len, PGconn *conn); static int pqSendSome(PGconn *conn, int len); -static int pqSocketCheck(PGconn *conn, int forRead, int forWrite, - time_t end_time); -static int pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time); +static int pqSocketCheck(PGconn *conn, int forRead, + int forWrite, int forConnCheck, time_t end_time); +static int pqSocketPoll(int sock, int forRead, + int forWrite, int forConnCheck, time_t end_time); /* * PQlibVersion: return the libpq version number @@ -993,7 +994,7 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time) { int result; - result = pqSocketCheck(conn, forRead, forWrite, finish_time); + result = pqSocketCheck(conn, forRead, forWrite, 0, finish_time); if (result < 0) return -1; /* errorMessage is already set */ @@ -1014,7 +1015,7 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time) int pqReadReady(PGconn *conn) { - return pqSocketCheck(conn, 1, 0, (time_t) 0); + return pqSocketCheck(conn, 1, 0, 0, (time_t) 0); } /* @@ -1024,19 +1025,52 @@ pqReadReady(PGconn *conn) int pqWriteReady(PGconn *conn) { - return pqSocketCheck(conn, 0, 1, (time_t) 0); + return pqSocketCheck(conn, 0, 1, 0, (time_t) 0); +} + +/* + * Check whether the socket peer closed the connection or not. + * + * Returns >0 if remote peer seems to be closed, 0 if it is valid, + * -1 if the input connection is bad or an error occurred. + */ +int +PQconnCheck(PGconn *conn) +{ + return pqSocketCheck(conn, 0, 0, 1, (time_t) 0); +} + +/* + * Check whether PQconnCheck() can work on this platform. + * + * Returns true (1) if this can use PQconnCheck(), otherwise false (0). + */ +int +PQconnCheckable(void) +{ +#if (defined(HAVE_POLL) && defined(POLLRDHUP)) + return true; +#else + return false; +#endif } /* * Checks a socket, using poll or select, for data to be read, written, - * or both. Returns >0 if one or more conditions are met, 0 if it timed - * out, -1 if an error occurred. + * or both. Moreover, this function can check the health of the connetion on + * some limited platforms if forConnCehck is specified. + * + * Returns >0 if one or more conditions are met, 0 if it timed out, -1 if an + * error occurred. Note that if 0 is returned and forConnCheck is requested, it + * means that the socket has not matched POLLRDHUP event and the connection is + * not closed by the socket peer. * * If SSL is in use, the SSL buffer is checked prior to checking the socket * for read data directly. */ static int -pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time) +pqSocketCheck(PGconn *conn, int forRead, + int forWrite, int forConnCheck, time_t end_time) { int result; @@ -1059,7 +1093,8 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time) /* We will retry as long as we get EINTR */ do - result = pqSocketPoll(conn->sock, forRead, forWrite, end_time); + result = pqSocketPoll(conn->sock, forRead, + forWrite, forConnCheck, end_time); while (result < 0 && SOCK_ERRNO == EINTR); if (result < 0) @@ -1076,15 +1111,21 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time) /* * Check a file descriptor for read and/or write data, possibly waiting. - * If neither forRead nor forWrite are set, immediately return a timeout - * condition (without waiting). Return >0 if condition is met, 0 - * if a timeout occurred, -1 if an error or interrupt occurred. + * Moreover, this function can check the health of connection on some limited + * platform if forConnCehck is specified. + * + * If neither forRead, forWrite nor forConnCheck are set, immediately return a + * timeout condition (without waiting). Return >0 if condition is met, 0 if a + * timeout occurred, -1 if an error or interrupt occurred. Note that if 0 is + * returned and forConnCheck is requested, it means that the socket has not + * matched POLLRDHUP event and the connection is not closed by the socket peer. * * Timeout is infinite if end_time is -1. Timeout is immediate (no blocking) * if end_time is 0 (or indeed, any time before now). */ static int -pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time) +pqSocketPoll(int sock, int forRead, + int forWrite, int forConnCheck, time_t end_time) { /* We use poll(2) if available, otherwise select(2) */ #ifdef HAVE_POLL @@ -1092,7 +1133,11 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time) int timeout_ms; if (!forRead && !forWrite) - return 0; + { + /* Connection check can be available on some limted platforms */ + if (!(forConnCheck && PQconnCheckable())) + return 0; + } input_fd.fd = sock; input_fd.events = POLLERR; @@ -1102,6 +1147,10 @@ pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time) input_fd.events |= POLLIN; if (forWrite) input_fd.events |= POLLOUT; +#if defined(POLLRDHUP) + if (forConnCheck) + input_fd.events |= POLLRDHUP; +#endif /* Compute appropriate timeout interval */ if (end_time == ((time_t) -1)) diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index f3d9220496..e1bd0cd7b7 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -648,6 +648,10 @@ extern int PQdsplen(const char *s, int encoding); /* Get encoding id from environment variable PGCLIENTENCODING */ extern int PQenv2encoding(void); +/* Check whether the postgres server is still alive or not */ +extern int PQconnCheck(PGconn *conn); +extern int PQconnCheckable(void); + /* === in fe-auth.c === */ extern char *PQencryptPassword(const char *passwd, const char *user); -- 2.27.0 [application/octet-stream] v35-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch (12.3K, ../../TYCPR01MB58700498838127C54BC9BD48F5B69@TYCPR01MB5870.jpnprd01.prod.outlook.com/3-v35-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch) download | inline diff: From d9ef6cf61b767c3d552c3adb0b65f9cef2bbe57a Mon Sep 17 00:00:00 2001 From: Hayato Kuroda <[email protected]> Date: Fri, 27 Jan 2023 03:17:30 +0000 Subject: [PATCH v35 2/3] postgres_fdw: add postgres_fdw_verify_connection_states This function can verify the status of connections that are establieshed by postgres_fdw. This check wil be done by PQconnCheck(), which means this is available only on systems that support the non-standard POLLRDHUP extension to the poll system call, including Linux. This returns true if existing connection is not closed by the remote peer. --- contrib/postgres_fdw/Makefile | 2 +- contrib/postgres_fdw/connection.c | 162 ++++++++++++++++++ contrib/postgres_fdw/meson.build | 1 + .../postgres_fdw/postgres_fdw--1.1--1.2.sql | 19 ++ contrib/postgres_fdw/postgres_fdw.control | 2 +- doc/src/sgml/postgres-fdw.sgml | 74 ++++++++ 6 files changed, 258 insertions(+), 2 deletions(-) create mode 100644 contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile index c1b0cad453..6d23768389 100644 --- a/contrib/postgres_fdw/Makefile +++ b/contrib/postgres_fdw/Makefile @@ -14,7 +14,7 @@ PG_CPPFLAGS = -I$(libpq_srcdir) SHLIB_LINK_INTERNAL = $(libpq) EXTENSION = postgres_fdw -DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql +DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql postgres_fdw--1.1--1.2.sql REGRESS = postgres_fdw diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 12b54f15cd..28f0775ffa 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -87,6 +87,9 @@ static bool xact_got_connection = false; PG_FUNCTION_INFO_V1(postgres_fdw_get_connections); PG_FUNCTION_INFO_V1(postgres_fdw_disconnect); PG_FUNCTION_INFO_V1(postgres_fdw_disconnect_all); +PG_FUNCTION_INFO_V1(postgres_fdw_verify_connection_states); +PG_FUNCTION_INFO_V1(postgres_fdw_verify_connection_states_all); +PG_FUNCTION_INFO_V1(postgres_fdw_can_verify_connection_states); /* prototypes of private functions */ static void make_new_connection(ConnCacheEntry *entry, UserMapping *user); @@ -117,6 +120,7 @@ static void pgfdw_finish_pre_subcommit_cleanup(List *pending_entries, int curlevel); static bool UserMappingPasswordRequired(UserMapping *user); static bool disconnect_cached_connections(Oid serverid); +static bool verify_cached_connections(Oid serverid, bool *checked); /* * Get a PGconn which can be used to execute queries on the remote PostgreSQL @@ -1832,3 +1836,161 @@ disconnect_cached_connections(Oid serverid) return result; } + +/* + * Workhorse to verify cached connections. + * + * This function scans all the connection cache entries and verifies the + * connections whose foreign server OID matches with the specified one. If + * InvalidOid is specified, it verifies all the cached connections. + * + * This function emits warnings if a disconnection is found. This returns true + * if disconnections cannot be found, otherwise returns false. + * + * checked will be set to true if PQconnCheck() is called at least once. + */ +static bool +verify_cached_connections(Oid serverid, bool *checked) +{ + HASH_SEQ_STATUS scan; + ConnCacheEntry *entry; + bool all = !OidIsValid(serverid); + bool result = true; + StringInfoData str; + + *checked = false; + + Assert(ConnectionHash); + + hash_seq_init(&scan, ConnectionHash); + while ((entry = (ConnCacheEntry *) hash_seq_search(&scan))) + { + /* Ignore cache entry if no open connection right now */ + if (!entry->conn) + continue; + + /* Skip if the entry is invalidated */ + if (entry->invalidated) + continue; + + if (all || entry->serverid == serverid) + { + if (PQconnCheck(entry->conn)) + { + /* A foreign server might be down, so construct a message */ + ForeignServer *server = GetForeignServer(entry->serverid); + + if (result) + { + /* + * Initialize and add a prefix if this is the first + * disconnection we found. + */ + initStringInfo(&str); + appendStringInfo(&str, "could not connect to server "); + + result = false; + } + else + appendStringInfo(&str, ", "); + + appendStringInfo(&str, "\"%s\"", server->servername); + } + + /* Set a flag to notify the caller */ + *checked = true; + } + } + + /* Raise a warning if disconnections are found */ + if (!result) + { + Assert(str.len); + ereport(WARNING, + (errcode(ERRCODE_CONNECTION_FAILURE), + errmsg("%s", str.data), + errdetail("Connection close is detected."), + errhint("Plsease check the health of server."))); + pfree(str.data); + } + + return result; +} + +/* + * Verify the specified cached connections. + * + * This function verifies the connections that are established by postgres_fdw + * from the local session to the foreign server with the given name. + * + * This function emits a warning if a disconnection is found. This returns true + * if existing connection is not closed by the remote peer. false is returned + * if the local session seems to be disconnected from other servers. NULL is + * returned if a valid connection to the specified foreign server is not + * established or this function is not available on this platform. + */ +Datum +postgres_fdw_verify_connection_states(PG_FUNCTION_ARGS) +{ + ForeignServer *server; + char *servername; + bool result, + checked = false; + + /* quick exit if the checking does not work well on this platfrom */ + if (!PQconnCheckable()) + PG_RETURN_NULL(); + + /* quick exit if connection cache has not been initialized yet */ + if (!ConnectionHash) + PG_RETURN_NULL(); + + servername = text_to_cstring(PG_GETARG_TEXT_PP(0)); + server = GetForeignServerByName(servername, false); + + result = verify_cached_connections(server->serverid, &checked); + + /* Return the result if checking function was called, otherwise NULL */ + if (checked) + PG_RETURN_BOOL(result); + else + PG_RETURN_NULL(); +} + +/* + * Verify all the cached connections. + * + * This function verifies all the connections that are established by postgres_fdw + * from the local session to the foreign servers. + */ +Datum +postgres_fdw_verify_connection_states_all(PG_FUNCTION_ARGS) +{ + bool result, + checked = false; + + /* quick exit if the checking does not work well on this platfrom */ + if (!PQconnCheckable()) + PG_RETURN_NULL(); + + /* quick exit if connection cache has not been initialized yet */ + if (!ConnectionHash) + PG_RETURN_NULL(); + + result = verify_cached_connections(InvalidOid, &checked); + + /* Return the result if checking function was called, otherwise NULL */ + if (checked) + PG_RETURN_BOOL(result); + else + PG_RETURN_NULL(); +} + +/* + * Check whether functions for verifying cached connections work well or not + */ +Datum +postgres_fdw_can_verify_connection_states(PG_FUNCTION_ARGS) +{ + PG_RETURN_BOOL(PQconnCheckable()); +} diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build index 2b451f165e..29118d47bb 100644 --- a/contrib/postgres_fdw/meson.build +++ b/contrib/postgres_fdw/meson.build @@ -26,6 +26,7 @@ install_data( 'postgres_fdw.control', 'postgres_fdw--1.0.sql', 'postgres_fdw--1.0--1.1.sql', + 'postgres_fdw--1.1--1.2.sql', kwargs: contrib_data_args, ) diff --git a/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql new file mode 100644 index 0000000000..a8556b4c9f --- /dev/null +++ b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql @@ -0,0 +1,19 @@ +/* contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION postgres_fdw UPDATE TO '1.2'" to load this file. \quit + +CREATE FUNCTION postgres_fdw_verify_connection_states (text) +RETURNS bool +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT PARALLEL RESTRICTED; + +CREATE FUNCTION postgres_fdw_verify_connection_states_all () +RETURNS bool +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT PARALLEL RESTRICTED; + +CREATE FUNCTION postgres_fdw_can_verify_connection_states () +RETURNS bool +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT PARALLEL SAFE; diff --git a/contrib/postgres_fdw/postgres_fdw.control b/contrib/postgres_fdw/postgres_fdw.control index d489382064..a4b800be4f 100644 --- a/contrib/postgres_fdw/postgres_fdw.control +++ b/contrib/postgres_fdw/postgres_fdw.control @@ -1,5 +1,5 @@ # postgres_fdw extension comment = 'foreign-data wrapper for remote PostgreSQL servers' -default_version = '1.1' +default_version = '1.2' module_pathname = '$libdir/postgres_fdw' relocatable = true diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 644f51835b..8cdb0d0a9d 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -826,6 +826,80 @@ postgres=# SELECT postgres_fdw_disconnect_all(); </para> </listitem> </varlistentry> + + <varlistentry> + <term><function>postgres_fdw_verify_connection_states(server_name text) returns boolean</function></term> + <listitem> + <para> + This function checks the status of remote connections established by + <filename>postgres_fdw</filename> from the local session to the foreign + server with the given name. This check is performed by polling the socket + and allows long-running transactions to be aborted sooner if the kernel + reports that the connection is closed. This function is currently + available only on systems that support the non-standard <symbol>POLLRDHUP</symbol> + extension to the <symbol>poll</symbol> system call, including Linux. This + returns <literal>true</literal> if existing connection is not closed by + the remote peer. <literal>false</literal> is returned if the local + session seems to be disconnected from other servers. <literal>NULL</literal> + is returned if a valid connection to the specified foreign server is not + established or this function is not available on this platform. If no + foreign server with the given name is found, an error is reported. + Example usage of the function: +<screen> +postgres=# SELECT postgres_fdw_verify_connection_states('loopback1'); + postgres_fdw_verify_connection_states +--------------------------------------- + t +</screen> + </para> + </listitem> + </varlistentry> + <varlistentry> + <term><function>postgres_fdw_verify_connection_states_all() returns boolean</function></term> + <listitem> + <para> + This function checks the status of all the remote connections established + by <filename>postgres_fdw</filename> from the local session to the + foreign servers. This check is performed by polling the socket and allows + long-running transactions to be aborted sooner if the kernel reports + that the connection is closed. This function is currently available only + on systems that support the non-standard <symbol>POLLRDHUP</symbol> + extension to the <symbol>poll</symbol> system call, including Linux. This + returns <literal>true</literal> if all connections are not closed by the + remote peer. <literal>false</literal> is returned if the local session + seems to be disconnected from at least one remote server. <literal>NULL</literal> + is returned if there are no established connections or this function is + not available on this platform. Example usage of the function: +<screen> +postgres=# SELECT postgres_fdw_verify_connection_states_all(); + postgres_fdw_verify_connection_states_all +------------------------------------------- + t +</screen> + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><function>postgres_fdw_can_verify_connection_states() returns boolean</function></term> + <listitem> + <para> + This function checks whether <function>postgres_fdw_verify_connection_states</function> + and <function>postgres_fdw_verify_connection_states_all</function> work + well or not. This returns <literal>true</literal> if it can be used, + otherwise returns <literal>false</literal>. Example usage of the + function: + +<screen> +postgres=# SELECT postgres_fdw_can_verify_connection_states(); + postgres_fdw_can_verify_connection_states +------------------------------------------- + t +</screen> + </para> + </listitem> + </varlistentry> + </variablelist> </sect2> -- 2.27.0 [application/octet-stream] v35-0003-add-test.patch (5.3K, ../../TYCPR01MB58700498838127C54BC9BD48F5B69@TYCPR01MB5870.jpnprd01.prod.outlook.com/4-v35-0003-add-test.patch) download | inline diff: From 987d157a13d8f2dc200b99b0a914f105a167e100 Mon Sep 17 00:00:00 2001 From: Hayato Kuroda <[email protected]> Date: Fri, 27 Jan 2023 03:17:36 +0000 Subject: [PATCH v35 3/3] add test --- .../postgres_fdw/expected/postgres_fdw.out | 64 +++++++++++++++++++ contrib/postgres_fdw/sql/postgres_fdw.sql | 62 ++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 04a3ef450c..6446f36092 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -11783,3 +11783,67 @@ ANALYZE analyze_table; -- cleanup DROP FOREIGN TABLE analyze_ftable; DROP TABLE analyze_table; +-- =================================================================== +-- test for postgres_fdw_verify_foreign_servers function +-- =================================================================== +-- Disable debug_discard_caches in order to manage remote connections +SET debug_discard_caches TO '0'; +-- -- The text of the error might vary across platforms, so only show SQLSTATE. +\set VERBOSITY sqlstate +-- Disconnect once and set application_name to an arbitrary value +SELECT 1 FROM postgres_fdw_disconnect_all(); + ?column? +---------- + 1 +(1 row) + +ALTER SERVER loopback OPTIONS (SET application_name 'healthcheck'); +-- Define procedure for testing verify functions +CREATE PROCEDURE test_verify_function(use_all boolean) AS $$ +DECLARE + can_verify boolean; + result boolean; +BEGIN + PERFORM 1 FROM ft1 LIMIT 1; + + -- Terminate the remote backend process + PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity + WHERE application_name = 'healthcheck'; + + -- Check whether we can do health check on this platform + SELECT INTO can_verify postgres_fdw_can_verify_connection_states(); + + -- If the checking can be done on this platform, call it + IF can_verify IS TRUE THEN + -- Set client_min_messages to ERROR temporary because the following + -- function only throws a WARNING on the supported platform. + SET LOCAL client_min_messages TO ERROR; + + IF use_all IS TRUE THEN + SELECT INTO result postgres_fdw_verify_connection_states_all(); + ELSE + SELECT INTO result postgres_fdw_verify_connection_states('loopback'); + END IF; + + RESET client_min_messages; + ELSE + result = false; + END IF; + + -- If result is FALSE, we succeeded to detect the disconnection or it could + -- not be done on this platform. Raise an message. + IF result IS FALSE THEN + RAISE INFO 'postgres_fdw_verify_connection_states_all() could detect the disconnection, or health check cannot be used on this platform'; + END IF; +END; +$$ LANGUAGE plpgsql; +-- ..And call above function +CALL test_verify_function(false); +INFO: 00000 +ERROR: 08006 +CALL test_verify_function(true); +INFO: 00000 +ERROR: 08006 +-- Clean up +\set VERBOSITY default +RESET debug_discard_caches; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 4f3088c03e..b413f96e32 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -3979,3 +3979,65 @@ ANALYZE analyze_table; -- cleanup DROP FOREIGN TABLE analyze_ftable; DROP TABLE analyze_table; + +-- =================================================================== +-- test for postgres_fdw_verify_foreign_servers function +-- =================================================================== + +-- Disable debug_discard_caches in order to manage remote connections +SET debug_discard_caches TO '0'; + +-- -- The text of the error might vary across platforms, so only show SQLSTATE. +\set VERBOSITY sqlstate + +-- Disconnect once and set application_name to an arbitrary value +SELECT 1 FROM postgres_fdw_disconnect_all(); +ALTER SERVER loopback OPTIONS (SET application_name 'healthcheck'); + +-- Define procedure for testing verify functions +CREATE PROCEDURE test_verify_function(use_all boolean) AS $$ +DECLARE + can_verify boolean; + result boolean; +BEGIN + PERFORM 1 FROM ft1 LIMIT 1; + + -- Terminate the remote backend process + PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity + WHERE application_name = 'healthcheck'; + + -- Check whether we can do health check on this platform + SELECT INTO can_verify postgres_fdw_can_verify_connection_states(); + + -- If the checking can be done on this platform, call it + IF can_verify IS TRUE THEN + -- Set client_min_messages to ERROR temporary because the following + -- function only throws a WARNING on the supported platform. + SET LOCAL client_min_messages TO ERROR; + + IF use_all IS TRUE THEN + SELECT INTO result postgres_fdw_verify_connection_states_all(); + ELSE + SELECT INTO result postgres_fdw_verify_connection_states('loopback'); + END IF; + + RESET client_min_messages; + ELSE + result = false; + END IF; + + -- If result is FALSE, we succeeded to detect the disconnection or it could + -- not be done on this platform. Raise an message. + IF result IS FALSE THEN + RAISE INFO 'postgres_fdw_verify_connection_states_all() could detect the disconnection, or health check cannot be used on this platform'; + END IF; +END; +$$ LANGUAGE plpgsql; + +-- ..And call above function +CALL test_verify_function(false); +CALL test_verify_function(true); + +-- Clean up +\set VERBOSITY default +RESET debug_discard_caches; -- 2.27.0 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [Proposal] Add foreign-server health checks infrastructure @ 2023-03-07 01:58 Katsuragi Yuta <[email protected]> parent: Hayato Kuroda (Fujitsu) <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Katsuragi Yuta @ 2023-03-07 01:58 UTC (permalink / raw) To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: 'Ted Yu' <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; [email protected]; Önder Kalacı <[email protected]>; Fujii Masao <[email protected]>; [email protected]; Kyotaro Horiguchi <[email protected]> Hi Kuroda-san, Thank you for updating the patch! I think we can update the status to ready for committer after this fix, if there is no objection. >> 7. the document of postgres_fdw_verify_connection_states_all >> <literal>NULL</literal> >> + is returned if the local session does not have connection >> caches, >> or this >> + function is not available on this platform. >> >> I think there is a case where a connection cache exists but valid >> connections do not exist and NULL is returned (disconnection case). >> Almost the same document as the postgres_fdw_verify_connection_states >> case (comment no.5) seems better. > > Yes, but completely same statement cannot be used because these is not > specified foreign server. How about: > NULL is returned if there are no established connections or this > function ... Yes, to align with the postgres_fdw_verify_connection_states() case, how about writing the connection is not valid. Like the following? 'NULL is returned if no valid connections are established or this function...' This is my comment for v35. Please check. 0002: 1. the comment of verify_cached_connections (I missed one minor point.) + * This function emits warnings if a disconnection is found. This returns true + * if disconnections cannot be found, otherwise returns false. I think false is returned only if disconnections are found and true is returned in all other cases. So, modifying the description like the following seems better. 'This returns false if disconnections are found, otherwise returns true.' regards, -- Katsuragi Yuta Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-03-07 01:58 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-12-10 03:40 [PATCH v6 3/3] Allow recovery via loadable modules. Nathan Bossart <[email protected]> 2023-03-06 11:56 RE: [Proposal] Add foreign-server health checks infrastructure Hayato Kuroda (Fujitsu) <[email protected]> 2023-03-07 01:58 ` Re: [Proposal] Add foreign-server health checks infrastructure Katsuragi Yuta <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox