agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feed[PATCH v2 3/3] restructure archive modules API
15+ messages / 2 participants
[nested] [flat]
* [PATCH v9 3/3] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 51 +++++++++++++++++++------
doc/src/sgml/archive-modules.sgml | 35 +++++++++++++----
src/backend/postmaster/pgarch.c | 23 ++++++-----
src/backend/postmaster/shell_archive.c | 29 ++++++++------
src/include/postmaster/archive_module.h | 21 +++++++---
src/include/postmaster/shell_archive.h | 2 +-
6 files changed, 114 insertions(+), 47 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 384336884d..9afa03c5e2 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +86,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +158,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +169,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..2db1b19216 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,23 +47,30 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. This function must return an
+ <structname>ArchiveModuleCallbacks</structname> struct filled with the
+ callback function pointers for individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
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">
@@ -73,6 +80,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs to have a state, it
+ can use <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +104,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +126,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +149,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 281d9fd8b7..99a07996f7 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -99,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -408,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -510,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -829,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -846,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -861,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 0f0558e155..54b02ca152 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -23,26 +23,31 @@
#include "postmaster/archive_module.h"
#include "postmaster/shell_archive.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -124,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index f5015ff95d..79e9d53534 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -17,6 +17,15 @@
*/
extern PGDLLIMPORT char *XLogArchiveLibrary;
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
/*
* Archive module callbacks
*
@@ -25,12 +34,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,8 +51,8 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
index 3cf737ce8e..be83c7715a 100644
--- a/src/include/postmaster/shell_archive.h
+++ b/src/include/postmaster/shell_archive.h
@@ -19,6 +19,6 @@
* and does not need to be loaded via a shared library, it has a special
* initialization function.
*/
-extern void shell_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--2oS5YaxWCcQjTEyO--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v5 3/3] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 53 +++++++++++++++++++------
doc/src/sgml/archive-modules.sgml | 33 ++++++++++++---
src/backend/postmaster/pgarch.c | 23 ++++++-----
src/backend/postmaster/shell_archive.c | 29 ++++++++------
src/include/postmaster/archive_module.h | 21 +++++++---
src/include/postmaster/shell_archive.h | 2 +-
6 files changed, 114 insertions(+), 47 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 87bbb2174d..74a51b9288 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,13 +86,30 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
-void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
{
AssertVariableIsOfType(&_PG_archive_module_init, ArchiveModuleInit);
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
+void
+basic_archive_startup(ArchiveModuleState *state)
+{
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -135,7 +160,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -146,10 +171,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..2ec5d12d97 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,23 +47,30 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
+ <function>_PG_archive_module_init</function>. This function must return an
+ ArchiveModuleCallbacks struct filled with the callback function pointers for
individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
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">
@@ -73,6 +80,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs to have a state, it
+ can use <literal>state->private_data</literal> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +104,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +126,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +149,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index ec47b2cc20..5931d5ffa0 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -99,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -408,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -510,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -829,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -846,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -861,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 35f88c1222..cc2585218d 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -23,28 +23,33 @@
#include "postmaster/archive_module.h"
#include "postmaster/shell_archive.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
AssertVariableIsOfType(&shell_archive_init, ArchiveModuleInit);
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -126,7 +131,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index f5015ff95d..79e9d53534 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -17,6 +17,15 @@
*/
extern PGDLLIMPORT char *XLogArchiveLibrary;
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
/*
* Archive module callbacks
*
@@ -25,12 +34,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,8 +51,8 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
index 3cf737ce8e..be83c7715a 100644
--- a/src/include/postmaster/shell_archive.h
+++ b/src/include/postmaster/shell_archive.h
@@ -19,6 +19,6 @@
* and does not need to be loaded via a shared library, it has a special
* initialization function.
*/
-extern void shell_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--+QahgC5+KEYLbs62--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v4 3/3] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 51 ++++++++++++++++++-------
doc/src/sgml/archive-modules.sgml | 33 +++++++++++++---
src/backend/postmaster/pgarch.c | 23 ++++++-----
src/backend/postmaster/shell_archive.c | 29 ++++++++------
src/include/postmaster/archive_module.h | 21 +++++++---
src/include/postmaster/shell_archive.h | 2 +-
6 files changed, 112 insertions(+), 47 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 87bbb2174d..3b4373b562 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,13 +86,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
-void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
{
AssertVariableIsOfType(&_PG_archive_module_init, ArchiveModuleInit);
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
+void
+basic_archive_startup(ArchiveModuleState *state)
+{
+ BasicArchiveData *data = palloc0(sizeof(BasicArchiveData));
+
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -135,7 +158,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -146,10 +169,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..2ec5d12d97 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,23 +47,30 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
+ <function>_PG_archive_module_init</function>. This function must return an
+ ArchiveModuleCallbacks struct filled with the callback function pointers for
individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
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">
@@ -73,6 +80,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs to have a state, it
+ can use <literal>state->private_data</literal> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +104,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +126,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +149,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index ec47b2cc20..5931d5ffa0 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -99,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -408,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -510,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -829,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -846,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -861,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 35f88c1222..cc2585218d 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -23,28 +23,33 @@
#include "postmaster/archive_module.h"
#include "postmaster/shell_archive.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
AssertVariableIsOfType(&shell_archive_init, ArchiveModuleInit);
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -126,7 +131,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index f5015ff95d..79e9d53534 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -17,6 +17,15 @@
*/
extern PGDLLIMPORT char *XLogArchiveLibrary;
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
/*
* Archive module callbacks
*
@@ -25,12 +34,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,8 +51,8 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
index 3cf737ce8e..be83c7715a 100644
--- a/src/include/postmaster/shell_archive.h
+++ b/src/include/postmaster/shell_archive.h
@@ -19,6 +19,6 @@
* and does not need to be loaded via a shared library, it has a special
* initialization function.
*/
-extern void shell_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--9amGYk9869ThD9tj--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v2 3/3] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 42 +++++++++++++++++--------
doc/src/sgml/archive-modules.sgml | 30 +++++++++++++-----
src/backend/postmaster/pgarch.c | 22 +++++++------
src/backend/postmaster/shell_archive.c | 29 ++++++++++-------
src/include/postmaster/archive_module.h | 14 +++++----
5 files changed, 89 insertions(+), 48 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 87bbb2174d..2b7680947f 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -41,14 +41,21 @@
PG_MODULE_MAGIC;
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void *basic_archive_startup(void);
+static bool basic_archive_configured(void *private_state);
+static bool basic_archive_file(const char *file, const char *path, void *private_state);
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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +74,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,13 +81,25 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
-void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
{
AssertVariableIsOfType(&_PG_archive_module_init, ArchiveModuleInit);
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
+void *
+basic_archive_startup(void)
+{
+ return (void *) AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -135,7 +150,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(void *private_state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -146,10 +161,11 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(const char *file, const char *path, void *private_state)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ MemoryContext basic_archive_context = (MemoryContext) private_state;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..8c4a3d9365 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,18 +47,18 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. This function must return a
+ struct filled with the callback function pointers for individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
Only the <function>archive_file_cb</function> callback is required. The
@@ -73,6 +73,22 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs a state, it should
+ return a pointer to the state. This pointer will be passed to each of the
+ module's other callbacks via the <literal>void *private_state</literal>
+ argument.
+
+<programlisting>
+typedef void *(*ArchiveStartupCB) (void);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +99,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (void *private_state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +121,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (const char *file, const char *path, void *private_state);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +144,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (void *private_state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index eca02d5e74..421425d0dd 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -98,7 +98,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static void *private_state = NULL;
/*
@@ -417,8 +418,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(private_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -519,7 +520,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(xlog, pathname, private_state);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -838,8 +839,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -855,12 +854,15 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ private_state = ArchiveCallbacks->startup_cb();
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -870,6 +872,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(private_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index b64297e3bb..dbd633477a 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -22,28 +22,33 @@
#include "pgstat.h"
#include "postmaster/archive_module.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(void *private_state);
+static bool shell_archive_file(const char *file, const char *path, void *private_state);
+static void shell_archive_shutdown(void *private_state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
AssertVariableIsOfType(&shell_archive_init, ArchiveModuleInit);
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(void *private_state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(const char *file, const char *path, void *private_state)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -125,7 +130,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(void *private_state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index 099050c1ca..b4ea4f5dcd 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -25,12 +25,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void *(*ArchiveStartupCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (void *private_state);
+typedef bool (*ArchiveFileCB) (const char *file, const char *path, void *private_state);
+typedef void (*ArchiveShutdownCB) (void *private_state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,15 +42,15 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
/*
* Since the logic for archiving 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_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _ARCHIVE_MODULE_H */
--
2.25.1
--cWoXeonUoKmBZSoM--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v3 3/3] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 51 ++++++++++++++++++-------
doc/src/sgml/archive-modules.sgml | 33 +++++++++++++---
src/backend/postmaster/pgarch.c | 23 ++++++-----
src/backend/postmaster/shell_archive.c | 29 ++++++++------
src/include/postmaster/archive_module.h | 21 +++++++---
src/include/postmaster/shell_archive.h | 2 +-
6 files changed, 112 insertions(+), 47 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 87bbb2174d..f36a1def15 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(const char *file, const char *path, ArchiveModuleState *state);
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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,13 +86,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
-void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
{
AssertVariableIsOfType(&_PG_archive_module_init, ArchiveModuleInit);
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
+void
+basic_archive_startup(ArchiveModuleState *state)
+{
+ BasicArchiveData *data = palloc0(sizeof(BasicArchiveData));
+
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -135,7 +158,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -146,10 +169,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(const char *file, const char *path, ArchiveModuleState *state)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..cc8a834a2e 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,23 +47,30 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
+ <function>_PG_archive_module_init</function>. This function must return an
+ ArchiveModuleCallbacks struct filled with the callback function pointers for
individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
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">
@@ -73,6 +80,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs to have a state, it
+ can use <literal>state->private_data</literal> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +104,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +126,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (const char *file, const char *path, ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +149,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index ec47b2cc20..253d456837 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -99,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -408,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -510,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(xlog, pathname, archive_module_state);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -829,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -846,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -861,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 35f88c1222..5eda7671e8 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -23,28 +23,33 @@
#include "postmaster/archive_module.h"
#include "postmaster/shell_archive.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(const char *file, const char *path, ArchiveModuleState *state);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
AssertVariableIsOfType(&shell_archive_init, ArchiveModuleInit);
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(const char *file, const char *path, ArchiveModuleState *state)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -126,7 +131,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index f5015ff95d..d07dd26c6e 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -17,6 +17,15 @@
*/
extern PGDLLIMPORT char *XLogArchiveLibrary;
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
/*
* Archive module callbacks
*
@@ -25,12 +34,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (const char *file, const char *path, ArchiveModuleState *state);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,8 +51,8 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
index f6943c0aba..66b2fda401 100644
--- a/src/include/postmaster/shell_archive.h
+++ b/src/include/postmaster/shell_archive.h
@@ -17,6 +17,6 @@
* and does not need to be loaded via a shared library, it has a special
* initialization function.
*/
-extern void shell_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--sm4nu43k4a2Rpi4c--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v6 3/3] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 53 +++++++++++++++++++------
doc/src/sgml/archive-modules.sgml | 35 ++++++++++++----
src/backend/postmaster/pgarch.c | 23 ++++++-----
src/backend/postmaster/shell_archive.c | 29 ++++++++------
src/include/postmaster/archive_module.h | 21 +++++++---
src/include/postmaster/shell_archive.h | 2 +-
6 files changed, 115 insertions(+), 48 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 87bbb2174d..74a51b9288 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,13 +86,30 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
-void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
{
AssertVariableIsOfType(&_PG_archive_module_init, ArchiveModuleInit);
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
+void
+basic_archive_startup(ArchiveModuleState *state)
+{
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -135,7 +160,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -146,10 +171,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..2db1b19216 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,23 +47,30 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. This function must return an
+ <structname>ArchiveModuleCallbacks</structname> struct filled with the
+ callback function pointers for individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
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">
@@ -73,6 +80,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs to have a state, it
+ can use <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +104,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +126,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +149,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 281d9fd8b7..99a07996f7 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -99,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -408,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -510,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -829,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -846,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -861,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 35f88c1222..cc2585218d 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -23,28 +23,33 @@
#include "postmaster/archive_module.h"
#include "postmaster/shell_archive.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
AssertVariableIsOfType(&shell_archive_init, ArchiveModuleInit);
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -126,7 +131,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index f5015ff95d..79e9d53534 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -17,6 +17,15 @@
*/
extern PGDLLIMPORT char *XLogArchiveLibrary;
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
/*
* Archive module callbacks
*
@@ -25,12 +34,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,8 +51,8 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
index 3cf737ce8e..be83c7715a 100644
--- a/src/include/postmaster/shell_archive.h
+++ b/src/include/postmaster/shell_archive.h
@@ -19,6 +19,6 @@
* and does not need to be loaded via a shared library, it has a special
* initialization function.
*/
-extern void shell_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--OXfL5xGRrasGEqWY--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v8 4/4] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 51 +++++++++++++++++++------
doc/src/sgml/archive-modules.sgml | 35 +++++++++++++----
src/backend/postmaster/pgarch.c | 23 ++++++-----
src/backend/postmaster/shell_archive.c | 29 ++++++++------
src/include/postmaster/archive_module.h | 21 +++++++---
src/include/postmaster/shell_archive.h | 2 +-
6 files changed, 114 insertions(+), 47 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 384336884d..9afa03c5e2 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +86,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +158,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +169,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..2db1b19216 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,23 +47,30 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. This function must return an
+ <structname>ArchiveModuleCallbacks</structname> struct filled with the
+ callback function pointers for individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
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">
@@ -73,6 +80,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs to have a state, it
+ can use <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +104,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +126,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +149,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 281d9fd8b7..99a07996f7 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -99,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -408,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -510,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -829,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -846,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -861,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 0f0558e155..54b02ca152 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -23,26 +23,31 @@
#include "postmaster/archive_module.h"
#include "postmaster/shell_archive.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -124,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index f5015ff95d..79e9d53534 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -17,6 +17,15 @@
*/
extern PGDLLIMPORT char *XLogArchiveLibrary;
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
/*
* Archive module callbacks
*
@@ -25,12 +34,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,8 +51,8 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
index 3cf737ce8e..be83c7715a 100644
--- a/src/include/postmaster/shell_archive.h
+++ b/src/include/postmaster/shell_archive.h
@@ -19,6 +19,6 @@
* and does not need to be loaded via a shared library, it has a special
* initialization function.
*/
-extern void shell_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--4Ckj6UjgE2iN1+kY--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v7 3/4] restructure archive modules API
@ 2023-01-31 22:58 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-01-31 22:58 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 53 +++++++++++++++++++------
doc/src/sgml/archive-modules.sgml | 35 ++++++++++++----
src/backend/postmaster/pgarch.c | 23 ++++++-----
src/backend/postmaster/shell_archive.c | 29 ++++++++------
src/include/postmaster/archive_module.h | 21 +++++++---
src/include/postmaster/shell_archive.h | 2 +-
6 files changed, 115 insertions(+), 48 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 87bbb2174d..74a51b9288 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,13 +86,30 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
-void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
{
AssertVariableIsOfType(&_PG_archive_module_init, ArchiveModuleInit);
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
+void
+basic_archive_startup(ArchiveModuleState *state)
+{
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -135,7 +160,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -146,10 +171,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..2db1b19216 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,23 +47,30 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. This function must return an
+ <structname>ArchiveModuleCallbacks</structname> struct filled with the
+ callback function pointers for individual actions.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
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">
@@ -73,6 +80,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module needs to have a state, it
+ can use <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +104,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +126,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +149,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 281d9fd8b7..99a07996f7 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -99,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveCallbacks;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -408,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveCallbacks.check_configured_cb != NULL &&
- !ArchiveCallbacks.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -510,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveCallbacks.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -829,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveCallbacks, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -846,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveCallbacks);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveCallbacks.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -861,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveCallbacks.shutdown_cb != NULL)
- ArchiveCallbacks.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 35f88c1222..cc2585218d 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -23,28 +23,33 @@
#include "postmaster/archive_module.h"
#include "postmaster/shell_archive.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
AssertVariableIsOfType(&shell_archive_init, ArchiveModuleInit);
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -126,7 +131,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
index f5015ff95d..79e9d53534 100644
--- a/src/include/postmaster/archive_module.h
+++ b/src/include/postmaster/archive_module.h
@@ -17,6 +17,15 @@
*/
extern PGDLLIMPORT char *XLogArchiveLibrary;
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
/*
* Archive module callbacks
*
@@ -25,12 +34,14 @@ extern PGDLLIMPORT char *XLogArchiveLibrary;
* For more information about the purpose of each callback, refer to the
* archive modules documentation.
*/
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
@@ -40,8 +51,8 @@ typedef struct ArchiveModuleCallbacks
* Type of the shared library symbol _PG_archive_module_init that is looked
* up when loading an archive library.
*/
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
index 3cf737ce8e..be83c7715a 100644
--- a/src/include/postmaster/shell_archive.h
+++ b/src/include/postmaster/shell_archive.h
@@ -19,6 +19,6 @@
* and does not need to be loaded via a shared library, it has a special
* initialization function.
*/
-extern void shell_archive_init(ArchiveModuleCallbacks *cb);
+extern const ArchiveModuleCallbacks *shell_archive_init(void);
#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--/04w6evG8XlLl3ft
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0004-remove-unnecessary-assertions-for-functions-with-.patch"
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v12 1/1] restructure archive modules API
@ 2023-02-09 21:49 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-02-09 21:49 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 85 ++++++++++++++++---
doc/src/sgml/archive-modules.sgml | 35 ++++++--
src/backend/Makefile | 2 +-
src/backend/archive/Makefile | 18 ++++
src/backend/archive/meson.build | 5 ++
.../{postmaster => archive}/shell_archive.c | 30 ++++---
src/backend/meson.build | 1 +
src/backend/postmaster/Makefile | 1 -
src/backend/postmaster/meson.build | 1 -
src/backend/postmaster/pgarch.c | 27 +++---
src/backend/utils/misc/guc_tables.c | 1 +
src/include/archive/archive_module.h | 58 +++++++++++++
src/include/archive/shell_archive.h | 22 +++++
src/include/postmaster/pgarch.h | 39 ---------
14 files changed, 239 insertions(+), 86 deletions(-)
create mode 100644 src/backend/archive/Makefile
create mode 100644 src/backend/archive/meson.build
rename src/backend/{postmaster => archive}/shell_archive.c (78%)
create mode 100644 src/include/archive/archive_module.h
create mode 100644 src/include/archive/shell_archive.h
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 36b7a4814a..d7c227a10b 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -30,9 +30,9 @@
#include <sys/time.h>
#include <unistd.h>
+#include "archive/archive_module.h"
#include "common/int.h"
#include "miscadmin.h"
-#include "postmaster/pgarch.h"
#include "storage/copydir.h"
#include "storage/fd.h"
#include "utils/guc.h"
@@ -40,14 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 void basic_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = basic_archive_shutdown
+};
/*
* _PG_init
@@ -67,10 +80,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +87,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +159,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +170,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
@@ -366,3 +394,34 @@ compare_files(const char *file1, const char *file2)
return ret;
}
+
+/*
+ * basic_archive_shutdown
+ *
+ * Frees our allocated state.
+ */
+static void
+basic_archive_shutdown(ArchiveModuleState *state)
+{
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context;
+
+ /*
+ * If we didn't get to storing the pointer to our allocated state, we don't
+ * have anything to clean up.
+ */
+ if (data == NULL)
+ return;
+
+ basic_archive_context = data->context;
+ Assert(CurrentMemoryContext != basic_archive_context);
+
+ if (MemoryContextIsValid(basic_archive_context))
+ MemoryContextDelete(basic_archive_context);
+
+ /*
+ * Finally, free the state.
+ */
+ pfree(data);
+ state->private_data = NULL;
+}
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..668eb34991 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,18 +47,22 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. The result of the function
+ must be a pointer to a struct of type
+ <structname>ArchiveModuleCallbacks</structname>, which contains everything
+ that the core code needs to know how to make use of the archive module. The
+ return value needs to be of server lifetime, which is typically achieved by
+ defining it as a <literal>static const</literal> variable in global scope.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
Only the <function>archive_file_cb</function> callback is required. The
@@ -73,6 +77,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module has state, it can use
+ <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +101,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +123,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -125,10 +143,11 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
process exits (e.g., after an error) or the value of
<xref linkend="guc-archive-library"/> changes. If no
<function>shutdown_cb</function> is defined, no special action is taken in
- these situations.
+ these situations. If the archive module has state, this callback should
+ free it to avoid leaks.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 86e6dcb792..e4bf0fe9c0 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -17,7 +17,7 @@ subdir = src/backend
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = access backup bootstrap catalog parser commands executor \
+SUBDIRS = access archive backup bootstrap catalog parser commands executor \
foreign lib libpq \
main nodes optimizer partitioning port postmaster \
regex replication rewrite \
diff --git a/src/backend/archive/Makefile b/src/backend/archive/Makefile
new file mode 100644
index 0000000000..8d2860d0df
--- /dev/null
+++ b/src/backend/archive/Makefile
@@ -0,0 +1,18 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for src/backend/archive
+#
+# IDENTIFICATION
+# src/backend/archive/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/archive
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+OBJS = \
+ shell_archive.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/archive/meson.build b/src/backend/archive/meson.build
new file mode 100644
index 0000000000..e194282931
--- /dev/null
+++ b/src/backend/archive/meson.build
@@ -0,0 +1,5 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+backend_sources += files(
+ 'shell_archive.c'
+)
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/archive/shell_archive.c
similarity index 78%
rename from src/backend/postmaster/shell_archive.c
rename to src/backend/archive/shell_archive.c
index 7771b951b7..48e8952c0c 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/archive/shell_archive.c
@@ -18,30 +18,36 @@
#include <sys/wait.h>
#include "access/xlog.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "common/percentrepl.h"
#include "pgstat.h"
-#include "postmaster/pgarch.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -123,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/backend/meson.build b/src/backend/meson.build
index b1db3ba75b..4fdd209b82 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -7,6 +7,7 @@ backend_link_with = [pgport_srv, common_srv]
generated_backend_sources = []
subdir('access')
+subdir('archive')
subdir('backup')
subdir('bootstrap')
subdir('catalog')
diff --git a/src/backend/postmaster/Makefile b/src/backend/postmaster/Makefile
index 3a794e54d6..047448b34e 100644
--- a/src/backend/postmaster/Makefile
+++ b/src/backend/postmaster/Makefile
@@ -22,7 +22,6 @@ OBJS = \
interrupt.o \
pgarch.o \
postmaster.o \
- shell_archive.o \
startup.o \
syslogger.o \
walwriter.o
diff --git a/src/backend/postmaster/meson.build b/src/backend/postmaster/meson.build
index 9079922de7..cda921fd10 100644
--- a/src/backend/postmaster/meson.build
+++ b/src/backend/postmaster/meson.build
@@ -10,7 +10,6 @@ backend_sources += files(
'interrupt.c',
'pgarch.c',
'postmaster.c',
- 'shell_archive.c',
'startup.c',
'syslogger.c',
'walwriter.c',
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..46af349564 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -31,6 +31,8 @@
#include "access/xlog.h"
#include "access/xlog_internal.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "lib/binaryheap.h"
#include "libpq/pqsignal.h"
#include "pgstat.h"
@@ -97,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveContext;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -406,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveContext.check_configured_cb != NULL &&
- !ArchiveContext.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -508,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveContext.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -814,7 +817,7 @@ HandlePgArchInterrupts(void)
/*
* LoadArchiveLibrary
*
- * Loads the archiving callbacks into our local ArchiveContext.
+ * Loads the archiving callbacks into our local ArchiveCallbacks.
*/
static void
LoadArchiveLibrary(void)
@@ -827,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -844,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveContext);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveContext.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -859,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveContext.shutdown_cb != NULL)
- ArchiveContext.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b46e3b8c55..91cb64602b 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -33,6 +33,7 @@
#include "access/xlog_internal.h"
#include "access/xlogprefetcher.h"
#include "access/xlogrecovery.h"
+#include "archive/archive_module.h"
#include "catalog/namespace.h"
#include "catalog/storage.h"
#include "commands/async.h"
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
new file mode 100644
index 0000000000..92ced4b222
--- /dev/null
+++ b/src/include/archive/archive_module.h
@@ -0,0 +1,58 @@
+/*-------------------------------------------------------------------------
+ *
+ * archive_module.h
+ * Exports for archive modules.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/archive_module.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _ARCHIVE_MODULE_H
+#define _ARCHIVE_MODULE_H
+
+/*
+ * The value of the archive_library GUC.
+ */
+extern PGDLLIMPORT char *XLogArchiveLibrary;
+
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
+/*
+ * Archive module callbacks
+ *
+ * These callback functions should be defined by archive libraries and returned
+ * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
+ * For more information about the purpose of each callback, refer to the
+ * archive modules documentation.
+ */
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
+
+typedef struct ArchiveModuleCallbacks
+{
+ ArchiveStartupCB startup_cb;
+ ArchiveCheckConfiguredCB check_configured_cb;
+ ArchiveFileCB archive_file_cb;
+ ArchiveShutdownCB shutdown_cb;
+} ArchiveModuleCallbacks;
+
+/*
+ * Type of the shared library symbol _PG_archive_module_init that is looked
+ * up when loading an archive library.
+ */
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
+
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
+
+#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/archive/shell_archive.h b/src/include/archive/shell_archive.h
new file mode 100644
index 0000000000..1c843c13c5
--- /dev/null
+++ b/src/include/archive/shell_archive.h
@@ -0,0 +1,22 @@
+/*-------------------------------------------------------------------------
+ *
+ * shell_archive.h
+ * Exports for archiving via shell.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/shell_archive.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _SHELL_ARCHIVE_H
+#define _SHELL_ARCHIVE_H
+
+/*
+ * Since the logic for archiving 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 const ArchiveModuleCallbacks *shell_archive_init(void);
+
+#endif /* _SHELL_ARCHIVE_H */
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index bcd51dfad6..3bd4fac71e 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -33,43 +33,4 @@ extern void PgArchiverMain(void) pg_attribute_noreturn();
extern void PgArchWakeup(void);
extern void PgArchForceDirScan(void);
-/*
- * The value of the archive_library GUC.
- */
-extern PGDLLIMPORT char *XLogArchiveLibrary;
-
-/*
- * Archive module callbacks
- *
- * These callback functions should be defined by archive libraries and returned
- * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
- * For more information about the purpose of each callback, refer to the
- * archive modules documentation.
- */
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
-
-typedef struct ArchiveModuleCallbacks
-{
- ArchiveCheckConfiguredCB check_configured_cb;
- ArchiveFileCB archive_file_cb;
- ArchiveShutdownCB shutdown_cb;
-} ArchiveModuleCallbacks;
-
-/*
- * Type of the shared library symbol _PG_archive_module_init that is looked
- * up when loading an archive library.
- */
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
-
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
-
-/*
- * Since the logic for archiving 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_archive_init(ArchiveModuleCallbacks *cb);
-
#endif /* _PGARCH_H */
--
2.25.1
--tThc/1wpZn/ma/RB--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v14 1/1] restructure archive modules API
@ 2023-02-09 21:49 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-02-09 21:49 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 85 ++++++++++++++++---
doc/src/sgml/archive-modules.sgml | 35 ++++++--
src/backend/Makefile | 2 +-
src/backend/archive/Makefile | 18 ++++
src/backend/archive/meson.build | 5 ++
.../{postmaster => archive}/shell_archive.c | 32 ++++---
src/backend/meson.build | 1 +
src/backend/postmaster/Makefile | 1 -
src/backend/postmaster/meson.build | 1 -
src/backend/postmaster/pgarch.c | 27 +++---
src/backend/utils/misc/guc_tables.c | 1 +
src/include/archive/archive_module.h | 58 +++++++++++++
src/include/archive/shell_archive.h | 24 ++++++
src/include/postmaster/pgarch.h | 39 ---------
14 files changed, 242 insertions(+), 87 deletions(-)
create mode 100644 src/backend/archive/Makefile
create mode 100644 src/backend/archive/meson.build
rename src/backend/{postmaster => archive}/shell_archive.c (77%)
create mode 100644 src/include/archive/archive_module.h
create mode 100644 src/include/archive/shell_archive.h
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 36b7a4814a..d7c227a10b 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -30,9 +30,9 @@
#include <sys/time.h>
#include <unistd.h>
+#include "archive/archive_module.h"
#include "common/int.h"
#include "miscadmin.h"
-#include "postmaster/pgarch.h"
#include "storage/copydir.h"
#include "storage/fd.h"
#include "utils/guc.h"
@@ -40,14 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 void basic_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = basic_archive_shutdown
+};
/*
* _PG_init
@@ -67,10 +80,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +87,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +159,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +170,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
@@ -366,3 +394,34 @@ compare_files(const char *file1, const char *file2)
return ret;
}
+
+/*
+ * basic_archive_shutdown
+ *
+ * Frees our allocated state.
+ */
+static void
+basic_archive_shutdown(ArchiveModuleState *state)
+{
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context;
+
+ /*
+ * If we didn't get to storing the pointer to our allocated state, we don't
+ * have anything to clean up.
+ */
+ if (data == NULL)
+ return;
+
+ basic_archive_context = data->context;
+ Assert(CurrentMemoryContext != basic_archive_context);
+
+ if (MemoryContextIsValid(basic_archive_context))
+ MemoryContextDelete(basic_archive_context);
+
+ /*
+ * Finally, free the state.
+ */
+ pfree(data);
+ state->private_data = NULL;
+}
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..668eb34991 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,18 +47,22 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. The result of the function
+ must be a pointer to a struct of type
+ <structname>ArchiveModuleCallbacks</structname>, which contains everything
+ that the core code needs to know how to make use of the archive module. The
+ return value needs to be of server lifetime, which is typically achieved by
+ defining it as a <literal>static const</literal> variable in global scope.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
Only the <function>archive_file_cb</function> callback is required. The
@@ -73,6 +77,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module has state, it can use
+ <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +101,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +123,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -125,10 +143,11 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
process exits (e.g., after an error) or the value of
<xref linkend="guc-archive-library"/> changes. If no
<function>shutdown_cb</function> is defined, no special action is taken in
- these situations.
+ these situations. If the archive module has state, this callback should
+ free it to avoid leaks.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 86e6dcb792..e4bf0fe9c0 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -17,7 +17,7 @@ subdir = src/backend
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = access backup bootstrap catalog parser commands executor \
+SUBDIRS = access archive backup bootstrap catalog parser commands executor \
foreign lib libpq \
main nodes optimizer partitioning port postmaster \
regex replication rewrite \
diff --git a/src/backend/archive/Makefile b/src/backend/archive/Makefile
new file mode 100644
index 0000000000..8d2860d0df
--- /dev/null
+++ b/src/backend/archive/Makefile
@@ -0,0 +1,18 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for src/backend/archive
+#
+# IDENTIFICATION
+# src/backend/archive/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/archive
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+OBJS = \
+ shell_archive.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/archive/meson.build b/src/backend/archive/meson.build
new file mode 100644
index 0000000000..e194282931
--- /dev/null
+++ b/src/backend/archive/meson.build
@@ -0,0 +1,5 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+backend_sources += files(
+ 'shell_archive.c'
+)
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/archive/shell_archive.c
similarity index 77%
rename from src/backend/postmaster/shell_archive.c
rename to src/backend/archive/shell_archive.c
index 7771b951b7..837da543f9 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/archive/shell_archive.c
@@ -9,7 +9,7 @@
* Copyright (c) 2022-2023, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/postmaster/shell_archive.c
+ * src/backend/archive/shell_archive.c
*
*-------------------------------------------------------------------------
*/
@@ -18,30 +18,36 @@
#include <sys/wait.h>
#include "access/xlog.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "common/percentrepl.h"
#include "pgstat.h"
-#include "postmaster/pgarch.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -123,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/backend/meson.build b/src/backend/meson.build
index b1db3ba75b..4fdd209b82 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -7,6 +7,7 @@ backend_link_with = [pgport_srv, common_srv]
generated_backend_sources = []
subdir('access')
+subdir('archive')
subdir('backup')
subdir('bootstrap')
subdir('catalog')
diff --git a/src/backend/postmaster/Makefile b/src/backend/postmaster/Makefile
index 3a794e54d6..047448b34e 100644
--- a/src/backend/postmaster/Makefile
+++ b/src/backend/postmaster/Makefile
@@ -22,7 +22,6 @@ OBJS = \
interrupt.o \
pgarch.o \
postmaster.o \
- shell_archive.o \
startup.o \
syslogger.o \
walwriter.o
diff --git a/src/backend/postmaster/meson.build b/src/backend/postmaster/meson.build
index 9079922de7..cda921fd10 100644
--- a/src/backend/postmaster/meson.build
+++ b/src/backend/postmaster/meson.build
@@ -10,7 +10,6 @@ backend_sources += files(
'interrupt.c',
'pgarch.c',
'postmaster.c',
- 'shell_archive.c',
'startup.c',
'syslogger.c',
'walwriter.c',
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..46af349564 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -31,6 +31,8 @@
#include "access/xlog.h"
#include "access/xlog_internal.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "lib/binaryheap.h"
#include "libpq/pqsignal.h"
#include "pgstat.h"
@@ -97,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveContext;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -406,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveContext.check_configured_cb != NULL &&
- !ArchiveContext.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -508,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveContext.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -814,7 +817,7 @@ HandlePgArchInterrupts(void)
/*
* LoadArchiveLibrary
*
- * Loads the archiving callbacks into our local ArchiveContext.
+ * Loads the archiving callbacks into our local ArchiveCallbacks.
*/
static void
LoadArchiveLibrary(void)
@@ -827,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -844,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveContext);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveContext.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -859,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveContext.shutdown_cb != NULL)
- ArchiveContext.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 43b9d92660..1c0583fe26 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -33,6 +33,7 @@
#include "access/xlog_internal.h"
#include "access/xlogprefetcher.h"
#include "access/xlogrecovery.h"
+#include "archive/archive_module.h"
#include "catalog/namespace.h"
#include "catalog/storage.h"
#include "commands/async.h"
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
new file mode 100644
index 0000000000..92ced4b222
--- /dev/null
+++ b/src/include/archive/archive_module.h
@@ -0,0 +1,58 @@
+/*-------------------------------------------------------------------------
+ *
+ * archive_module.h
+ * Exports for archive modules.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/archive_module.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _ARCHIVE_MODULE_H
+#define _ARCHIVE_MODULE_H
+
+/*
+ * The value of the archive_library GUC.
+ */
+extern PGDLLIMPORT char *XLogArchiveLibrary;
+
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
+/*
+ * Archive module callbacks
+ *
+ * These callback functions should be defined by archive libraries and returned
+ * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
+ * For more information about the purpose of each callback, refer to the
+ * archive modules documentation.
+ */
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
+
+typedef struct ArchiveModuleCallbacks
+{
+ ArchiveStartupCB startup_cb;
+ ArchiveCheckConfiguredCB check_configured_cb;
+ ArchiveFileCB archive_file_cb;
+ ArchiveShutdownCB shutdown_cb;
+} ArchiveModuleCallbacks;
+
+/*
+ * Type of the shared library symbol _PG_archive_module_init that is looked
+ * up when loading an archive library.
+ */
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
+
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
+
+#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/archive/shell_archive.h b/src/include/archive/shell_archive.h
new file mode 100644
index 0000000000..9de6f769f1
--- /dev/null
+++ b/src/include/archive/shell_archive.h
@@ -0,0 +1,24 @@
+/*-------------------------------------------------------------------------
+ *
+ * shell_archive.h
+ * Exports for archiving via shell.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/shell_archive.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _SHELL_ARCHIVE_H
+#define _SHELL_ARCHIVE_H
+
+#include "archive/archive_module.h"
+
+/*
+ * Since the logic for archiving 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 const ArchiveModuleCallbacks *shell_archive_init(void);
+
+#endif /* _SHELL_ARCHIVE_H */
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index bcd51dfad6..3bd4fac71e 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -33,43 +33,4 @@ extern void PgArchiverMain(void) pg_attribute_noreturn();
extern void PgArchWakeup(void);
extern void PgArchForceDirScan(void);
-/*
- * The value of the archive_library GUC.
- */
-extern PGDLLIMPORT char *XLogArchiveLibrary;
-
-/*
- * Archive module callbacks
- *
- * These callback functions should be defined by archive libraries and returned
- * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
- * For more information about the purpose of each callback, refer to the
- * archive modules documentation.
- */
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
-
-typedef struct ArchiveModuleCallbacks
-{
- ArchiveCheckConfiguredCB check_configured_cb;
- ArchiveFileCB archive_file_cb;
- ArchiveShutdownCB shutdown_cb;
-} ArchiveModuleCallbacks;
-
-/*
- * Type of the shared library symbol _PG_archive_module_init that is looked
- * up when loading an archive library.
- */
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
-
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
-
-/*
- * Since the logic for archiving 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_archive_init(ArchiveModuleCallbacks *cb);
-
#endif /* _PGARCH_H */
--
2.25.1
--DocE+STaALJfprDB--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v10 1/1] restructure archive modules API
@ 2023-02-09 21:49 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-02-09 21:49 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 91 +++++++++++++++++++++----
doc/src/sgml/archive-modules.sgml | 35 +++++++---
src/backend/postmaster/pgarch.c | 27 +++++---
src/backend/postmaster/shell_archive.c | 34 +++++----
src/backend/utils/misc/guc_tables.c | 1 +
src/include/postmaster/archive_module.h | 58 ++++++++++++++++
src/include/postmaster/pgarch.h | 39 -----------
src/include/postmaster/shell_archive.h | 24 +++++++
8 files changed, 224 insertions(+), 85 deletions(-)
create mode 100644 src/include/postmaster/archive_module.h
create mode 100644 src/include/postmaster/shell_archive.h
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 36b7a4814a..e4742c9c94 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -32,7 +32,7 @@
#include "common/int.h"
#include "miscadmin.h"
-#include "postmaster/pgarch.h"
+#include "postmaster/archive_module.h"
#include "storage/copydir.h"
#include "storage/fd.h"
#include "utils/guc.h"
@@ -40,14 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 void basic_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = basic_archive_shutdown
+};
/*
* _PG_init
@@ -67,10 +80,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +87,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +159,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +170,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
@@ -366,3 +394,40 @@ compare_files(const char *file1, const char *file2)
return ret;
}
+
+/*
+ * basic_archive_shutdown
+ *
+ * Frees our allocated state.
+ */
+static void
+basic_archive_shutdown(ArchiveModuleState *state)
+{
+ BasicArchiveData *data;
+ MemoryContext basic_archive_context;
+
+ /*
+ * If we didn't get to storing the pointer to our allocated state, we don't
+ * have anything to clean up.
+ */
+ data = (BasicArchiveData *) (state->private_data);
+ if (data == NULL)
+ return;
+
+ /*
+ * If called within basic_archive_context, switch out of it before we
+ * delete it.
+ */
+ basic_archive_context = data->context;
+ if (CurrentMemoryContext == basic_archive_context)
+ MemoryContextSwitchTo(TopMemoryContext);
+
+ if (MemoryContextIsValid(basic_archive_context))
+ MemoryContextDelete(basic_archive_context);
+
+ /*
+ * Finally, free the state.
+ */
+ pfree(data);
+ state->private_data = NULL;
+}
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..668eb34991 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,18 +47,22 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. The result of the function
+ must be a pointer to a struct of type
+ <structname>ArchiveModuleCallbacks</structname>, which contains everything
+ that the core code needs to know how to make use of the archive module. The
+ return value needs to be of server lifetime, which is typically achieved by
+ defining it as a <literal>static const</literal> variable in global scope.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
Only the <function>archive_file_cb</function> callback is required. The
@@ -73,6 +77,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module has state, it can use
+ <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +101,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +123,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -125,10 +143,11 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
process exits (e.g., after an error) or the value of
<xref linkend="guc-archive-library"/> changes. If no
<function>shutdown_cb</function> is defined, no special action is taken in
- these situations.
+ these situations. If the archive module has state, this callback should
+ free it to avoid leaks.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..99a07996f7 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -34,8 +34,10 @@
#include "lib/binaryheap.h"
#include "libpq/pqsignal.h"
#include "pgstat.h"
+#include "postmaster/archive_module.h"
#include "postmaster/interrupt.h"
#include "postmaster/pgarch.h"
+#include "postmaster/shell_archive.h"
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/latch.h"
@@ -97,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveContext;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -406,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveContext.check_configured_cb != NULL &&
- !ArchiveContext.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -508,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveContext.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -814,7 +817,7 @@ HandlePgArchInterrupts(void)
/*
* LoadArchiveLibrary
*
- * Loads the archiving callbacks into our local ArchiveContext.
+ * Loads the archiving callbacks into our local ArchiveCallbacks.
*/
static void
LoadArchiveLibrary(void)
@@ -827,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -844,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveContext);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveContext.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -859,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveContext.shutdown_cb != NULL)
- ArchiveContext.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/postmaster/shell_archive.c
index 7771b951b7..54b02ca152 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/postmaster/shell_archive.c
@@ -20,28 +20,34 @@
#include "access/xlog.h"
#include "common/percentrepl.h"
#include "pgstat.h"
-#include "postmaster/pgarch.h"
-
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
-
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+#include "postmaster/archive_module.h"
+#include "postmaster/shell_archive.h"
+
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -123,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b46e3b8c55..c3ee83b691 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -52,6 +52,7 @@
#include "parser/parse_expr.h"
#include "parser/parser.h"
#include "pgstat.h"
+#include "postmaster/archive_module.h"
#include "postmaster/autovacuum.h"
#include "postmaster/bgworker_internals.h"
#include "postmaster/bgwriter.h"
diff --git a/src/include/postmaster/archive_module.h b/src/include/postmaster/archive_module.h
new file mode 100644
index 0000000000..79e9d53534
--- /dev/null
+++ b/src/include/postmaster/archive_module.h
@@ -0,0 +1,58 @@
+/*-------------------------------------------------------------------------
+ *
+ * archive_module.h
+ * Exports for archive modules.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/postmaster/archive_module.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _ARCHIVE_MODULE_H
+#define _ARCHIVE_MODULE_H
+
+/*
+ * The value of the archive_library GUC.
+ */
+extern PGDLLIMPORT char *XLogArchiveLibrary;
+
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
+/*
+ * Archive module callbacks
+ *
+ * These callback functions should be defined by archive libraries and returned
+ * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
+ * For more information about the purpose of each callback, refer to the
+ * archive modules documentation.
+ */
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
+
+typedef struct ArchiveModuleCallbacks
+{
+ ArchiveStartupCB startup_cb;
+ ArchiveCheckConfiguredCB check_configured_cb;
+ ArchiveFileCB archive_file_cb;
+ ArchiveShutdownCB shutdown_cb;
+} ArchiveModuleCallbacks;
+
+/*
+ * Type of the shared library symbol _PG_archive_module_init that is looked
+ * up when loading an archive library.
+ */
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
+
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
+
+#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index bcd51dfad6..3bd4fac71e 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -33,43 +33,4 @@ extern void PgArchiverMain(void) pg_attribute_noreturn();
extern void PgArchWakeup(void);
extern void PgArchForceDirScan(void);
-/*
- * The value of the archive_library GUC.
- */
-extern PGDLLIMPORT char *XLogArchiveLibrary;
-
-/*
- * Archive module callbacks
- *
- * These callback functions should be defined by archive libraries and returned
- * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
- * For more information about the purpose of each callback, refer to the
- * archive modules documentation.
- */
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
-
-typedef struct ArchiveModuleCallbacks
-{
- ArchiveCheckConfiguredCB check_configured_cb;
- ArchiveFileCB archive_file_cb;
- ArchiveShutdownCB shutdown_cb;
-} ArchiveModuleCallbacks;
-
-/*
- * Type of the shared library symbol _PG_archive_module_init that is looked
- * up when loading an archive library.
- */
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
-
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
-
-/*
- * Since the logic for archiving 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_archive_init(ArchiveModuleCallbacks *cb);
-
#endif /* _PGARCH_H */
diff --git a/src/include/postmaster/shell_archive.h b/src/include/postmaster/shell_archive.h
new file mode 100644
index 0000000000..be83c7715a
--- /dev/null
+++ b/src/include/postmaster/shell_archive.h
@@ -0,0 +1,24 @@
+/*-------------------------------------------------------------------------
+ *
+ * shell_archive.h
+ * Exports for archiving via shell.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/postmaster/shell_archive.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _SHELL_ARCHIVE_H
+#define _SHELL_ARCHIVE_H
+
+#include "postmaster/archive_module.h"
+
+/*
+ * Since the logic for archiving 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 const ArchiveModuleCallbacks *shell_archive_init(void);
+
+#endif /* _SHELL_ARCHIVE_H */
--
2.25.1
--4Ckj6UjgE2iN1+kY--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v13 1/1] restructure archive modules API
@ 2023-02-09 21:49 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-02-09 21:49 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 85 ++++++++++++++++---
doc/src/sgml/archive-modules.sgml | 35 ++++++--
src/backend/Makefile | 2 +-
src/backend/archive/Makefile | 18 ++++
src/backend/archive/meson.build | 5 ++
.../{postmaster => archive}/shell_archive.c | 30 ++++---
src/backend/meson.build | 1 +
src/backend/postmaster/Makefile | 1 -
src/backend/postmaster/meson.build | 1 -
src/backend/postmaster/pgarch.c | 27 +++---
src/backend/utils/misc/guc_tables.c | 1 +
src/include/archive/archive_module.h | 58 +++++++++++++
src/include/archive/shell_archive.h | 24 ++++++
src/include/postmaster/pgarch.h | 39 ---------
14 files changed, 241 insertions(+), 86 deletions(-)
create mode 100644 src/backend/archive/Makefile
create mode 100644 src/backend/archive/meson.build
rename src/backend/{postmaster => archive}/shell_archive.c (78%)
create mode 100644 src/include/archive/archive_module.h
create mode 100644 src/include/archive/shell_archive.h
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 36b7a4814a..d7c227a10b 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -30,9 +30,9 @@
#include <sys/time.h>
#include <unistd.h>
+#include "archive/archive_module.h"
#include "common/int.h"
#include "miscadmin.h"
-#include "postmaster/pgarch.h"
#include "storage/copydir.h"
#include "storage/fd.h"
#include "utils/guc.h"
@@ -40,14 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 void basic_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = basic_archive_shutdown
+};
/*
* _PG_init
@@ -67,10 +80,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +87,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +159,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +170,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
@@ -366,3 +394,34 @@ compare_files(const char *file1, const char *file2)
return ret;
}
+
+/*
+ * basic_archive_shutdown
+ *
+ * Frees our allocated state.
+ */
+static void
+basic_archive_shutdown(ArchiveModuleState *state)
+{
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context;
+
+ /*
+ * If we didn't get to storing the pointer to our allocated state, we don't
+ * have anything to clean up.
+ */
+ if (data == NULL)
+ return;
+
+ basic_archive_context = data->context;
+ Assert(CurrentMemoryContext != basic_archive_context);
+
+ if (MemoryContextIsValid(basic_archive_context))
+ MemoryContextDelete(basic_archive_context);
+
+ /*
+ * Finally, free the state.
+ */
+ pfree(data);
+ state->private_data = NULL;
+}
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..668eb34991 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,18 +47,22 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. The result of the function
+ must be a pointer to a struct of type
+ <structname>ArchiveModuleCallbacks</structname>, which contains everything
+ that the core code needs to know how to make use of the archive module. The
+ return value needs to be of server lifetime, which is typically achieved by
+ defining it as a <literal>static const</literal> variable in global scope.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
Only the <function>archive_file_cb</function> callback is required. The
@@ -73,6 +77,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module has state, it can use
+ <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +101,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +123,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -125,10 +143,11 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
process exits (e.g., after an error) or the value of
<xref linkend="guc-archive-library"/> changes. If no
<function>shutdown_cb</function> is defined, no special action is taken in
- these situations.
+ these situations. If the archive module has state, this callback should
+ free it to avoid leaks.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 86e6dcb792..e4bf0fe9c0 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -17,7 +17,7 @@ subdir = src/backend
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = access backup bootstrap catalog parser commands executor \
+SUBDIRS = access archive backup bootstrap catalog parser commands executor \
foreign lib libpq \
main nodes optimizer partitioning port postmaster \
regex replication rewrite \
diff --git a/src/backend/archive/Makefile b/src/backend/archive/Makefile
new file mode 100644
index 0000000000..8d2860d0df
--- /dev/null
+++ b/src/backend/archive/Makefile
@@ -0,0 +1,18 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for src/backend/archive
+#
+# IDENTIFICATION
+# src/backend/archive/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/archive
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+OBJS = \
+ shell_archive.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/archive/meson.build b/src/backend/archive/meson.build
new file mode 100644
index 0000000000..e194282931
--- /dev/null
+++ b/src/backend/archive/meson.build
@@ -0,0 +1,5 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+backend_sources += files(
+ 'shell_archive.c'
+)
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/archive/shell_archive.c
similarity index 78%
rename from src/backend/postmaster/shell_archive.c
rename to src/backend/archive/shell_archive.c
index 7771b951b7..48e8952c0c 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/archive/shell_archive.c
@@ -18,30 +18,36 @@
#include <sys/wait.h>
#include "access/xlog.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "common/percentrepl.h"
#include "pgstat.h"
-#include "postmaster/pgarch.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -123,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/backend/meson.build b/src/backend/meson.build
index b1db3ba75b..4fdd209b82 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -7,6 +7,7 @@ backend_link_with = [pgport_srv, common_srv]
generated_backend_sources = []
subdir('access')
+subdir('archive')
subdir('backup')
subdir('bootstrap')
subdir('catalog')
diff --git a/src/backend/postmaster/Makefile b/src/backend/postmaster/Makefile
index 3a794e54d6..047448b34e 100644
--- a/src/backend/postmaster/Makefile
+++ b/src/backend/postmaster/Makefile
@@ -22,7 +22,6 @@ OBJS = \
interrupt.o \
pgarch.o \
postmaster.o \
- shell_archive.o \
startup.o \
syslogger.o \
walwriter.o
diff --git a/src/backend/postmaster/meson.build b/src/backend/postmaster/meson.build
index 9079922de7..cda921fd10 100644
--- a/src/backend/postmaster/meson.build
+++ b/src/backend/postmaster/meson.build
@@ -10,7 +10,6 @@ backend_sources += files(
'interrupt.c',
'pgarch.c',
'postmaster.c',
- 'shell_archive.c',
'startup.c',
'syslogger.c',
'walwriter.c',
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..46af349564 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -31,6 +31,8 @@
#include "access/xlog.h"
#include "access/xlog_internal.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "lib/binaryheap.h"
#include "libpq/pqsignal.h"
#include "pgstat.h"
@@ -97,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveContext;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -406,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveContext.check_configured_cb != NULL &&
- !ArchiveContext.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -508,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveContext.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -814,7 +817,7 @@ HandlePgArchInterrupts(void)
/*
* LoadArchiveLibrary
*
- * Loads the archiving callbacks into our local ArchiveContext.
+ * Loads the archiving callbacks into our local ArchiveCallbacks.
*/
static void
LoadArchiveLibrary(void)
@@ -827,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -844,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveContext);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveContext.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -859,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveContext.shutdown_cb != NULL)
- ArchiveContext.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b46e3b8c55..91cb64602b 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -33,6 +33,7 @@
#include "access/xlog_internal.h"
#include "access/xlogprefetcher.h"
#include "access/xlogrecovery.h"
+#include "archive/archive_module.h"
#include "catalog/namespace.h"
#include "catalog/storage.h"
#include "commands/async.h"
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
new file mode 100644
index 0000000000..92ced4b222
--- /dev/null
+++ b/src/include/archive/archive_module.h
@@ -0,0 +1,58 @@
+/*-------------------------------------------------------------------------
+ *
+ * archive_module.h
+ * Exports for archive modules.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/archive_module.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _ARCHIVE_MODULE_H
+#define _ARCHIVE_MODULE_H
+
+/*
+ * The value of the archive_library GUC.
+ */
+extern PGDLLIMPORT char *XLogArchiveLibrary;
+
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
+/*
+ * Archive module callbacks
+ *
+ * These callback functions should be defined by archive libraries and returned
+ * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
+ * For more information about the purpose of each callback, refer to the
+ * archive modules documentation.
+ */
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
+
+typedef struct ArchiveModuleCallbacks
+{
+ ArchiveStartupCB startup_cb;
+ ArchiveCheckConfiguredCB check_configured_cb;
+ ArchiveFileCB archive_file_cb;
+ ArchiveShutdownCB shutdown_cb;
+} ArchiveModuleCallbacks;
+
+/*
+ * Type of the shared library symbol _PG_archive_module_init that is looked
+ * up when loading an archive library.
+ */
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
+
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
+
+#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/archive/shell_archive.h b/src/include/archive/shell_archive.h
new file mode 100644
index 0000000000..9de6f769f1
--- /dev/null
+++ b/src/include/archive/shell_archive.h
@@ -0,0 +1,24 @@
+/*-------------------------------------------------------------------------
+ *
+ * shell_archive.h
+ * Exports for archiving via shell.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/shell_archive.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _SHELL_ARCHIVE_H
+#define _SHELL_ARCHIVE_H
+
+#include "archive/archive_module.h"
+
+/*
+ * Since the logic for archiving 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 const ArchiveModuleCallbacks *shell_archive_init(void);
+
+#endif /* _SHELL_ARCHIVE_H */
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index bcd51dfad6..3bd4fac71e 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -33,43 +33,4 @@ extern void PgArchiverMain(void) pg_attribute_noreturn();
extern void PgArchWakeup(void);
extern void PgArchForceDirScan(void);
-/*
- * The value of the archive_library GUC.
- */
-extern PGDLLIMPORT char *XLogArchiveLibrary;
-
-/*
- * Archive module callbacks
- *
- * These callback functions should be defined by archive libraries and returned
- * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
- * For more information about the purpose of each callback, refer to the
- * archive modules documentation.
- */
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
-
-typedef struct ArchiveModuleCallbacks
-{
- ArchiveCheckConfiguredCB check_configured_cb;
- ArchiveFileCB archive_file_cb;
- ArchiveShutdownCB shutdown_cb;
-} ArchiveModuleCallbacks;
-
-/*
- * Type of the shared library symbol _PG_archive_module_init that is looked
- * up when loading an archive library.
- */
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
-
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
-
-/*
- * Since the logic for archiving 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_archive_init(ArchiveModuleCallbacks *cb);
-
#endif /* _PGARCH_H */
--
2.25.1
--7AUc2qLy4jB3hD7Z--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v15 1/1] restructure archive modules API
@ 2023-02-09 21:49 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-02-09 21:49 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 86 ++++++++++++++++---
doc/src/sgml/archive-modules.sgml | 35 ++++++--
src/backend/Makefile | 2 +-
src/backend/archive/Makefile | 18 ++++
src/backend/archive/meson.build | 5 ++
.../{postmaster => archive}/shell_archive.c | 32 ++++---
src/backend/meson.build | 1 +
src/backend/postmaster/Makefile | 1 -
src/backend/postmaster/meson.build | 1 -
src/backend/postmaster/pgarch.c | 27 +++---
src/backend/utils/misc/guc_tables.c | 1 +
src/include/archive/archive_module.h | 58 +++++++++++++
src/include/archive/shell_archive.h | 24 ++++++
src/include/postmaster/pgarch.h | 39 ---------
14 files changed, 243 insertions(+), 87 deletions(-)
create mode 100644 src/backend/archive/Makefile
create mode 100644 src/backend/archive/meson.build
rename src/backend/{postmaster => archive}/shell_archive.c (77%)
create mode 100644 src/include/archive/archive_module.h
create mode 100644 src/include/archive/shell_archive.h
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 36b7a4814a..cd852888ce 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -30,9 +30,9 @@
#include <sys/time.h>
#include <unistd.h>
+#include "archive/archive_module.h"
#include "common/int.h"
#include "miscadmin.h"
-#include "postmaster/pgarch.h"
#include "storage/copydir.h"
#include "storage/fd.h"
#include "utils/guc.h"
@@ -40,14 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 void basic_archive_shutdown(ArchiveModuleState *state);
+
+static const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = basic_archive_shutdown
+};
/*
* _PG_init
@@ -67,10 +80,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +87,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +159,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +170,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) state->private_data;
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
@@ -366,3 +394,35 @@ compare_files(const char *file1, const char *file2)
return ret;
}
+
+/*
+ * basic_archive_shutdown
+ *
+ * Frees our allocated state.
+ */
+static void
+basic_archive_shutdown(ArchiveModuleState *state)
+{
+ BasicArchiveData *data = (BasicArchiveData *) state->private_data;
+ MemoryContext basic_archive_context;
+
+ /*
+ * If we didn't get to storing the pointer to our allocated state, we don't
+ * have anything to clean up.
+ */
+ if (data == NULL)
+ return;
+
+ basic_archive_context = data->context;
+ Assert(CurrentMemoryContext != basic_archive_context);
+
+ if (MemoryContextIsValid(basic_archive_context))
+ MemoryContextDelete(basic_archive_context);
+ data->context = NULL;
+
+ /*
+ * Finally, free the state.
+ */
+ pfree(data);
+ state->private_data = NULL;
+}
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..668eb34991 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,18 +47,22 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. The result of the function
+ must be a pointer to a struct of type
+ <structname>ArchiveModuleCallbacks</structname>, which contains everything
+ that the core code needs to know how to make use of the archive module. The
+ return value needs to be of server lifetime, which is typically achieved by
+ defining it as a <literal>static const</literal> variable in global scope.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
Only the <function>archive_file_cb</function> callback is required. The
@@ -73,6 +77,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module has state, it can use
+ <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +101,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +123,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -125,10 +143,11 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
process exits (e.g., after an error) or the value of
<xref linkend="guc-archive-library"/> changes. If no
<function>shutdown_cb</function> is defined, no special action is taken in
- these situations.
+ these situations. If the archive module has state, this callback should
+ free it to avoid leaks.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 86e6dcb792..e4bf0fe9c0 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -17,7 +17,7 @@ subdir = src/backend
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = access backup bootstrap catalog parser commands executor \
+SUBDIRS = access archive backup bootstrap catalog parser commands executor \
foreign lib libpq \
main nodes optimizer partitioning port postmaster \
regex replication rewrite \
diff --git a/src/backend/archive/Makefile b/src/backend/archive/Makefile
new file mode 100644
index 0000000000..8d2860d0df
--- /dev/null
+++ b/src/backend/archive/Makefile
@@ -0,0 +1,18 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for src/backend/archive
+#
+# IDENTIFICATION
+# src/backend/archive/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/archive
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+OBJS = \
+ shell_archive.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/archive/meson.build b/src/backend/archive/meson.build
new file mode 100644
index 0000000000..e194282931
--- /dev/null
+++ b/src/backend/archive/meson.build
@@ -0,0 +1,5 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+backend_sources += files(
+ 'shell_archive.c'
+)
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/archive/shell_archive.c
similarity index 77%
rename from src/backend/postmaster/shell_archive.c
rename to src/backend/archive/shell_archive.c
index 7771b951b7..837da543f9 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/archive/shell_archive.c
@@ -9,7 +9,7 @@
* Copyright (c) 2022-2023, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/postmaster/shell_archive.c
+ * src/backend/archive/shell_archive.c
*
*-------------------------------------------------------------------------
*/
@@ -18,30 +18,36 @@
#include <sys/wait.h>
#include "access/xlog.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "common/percentrepl.h"
#include "pgstat.h"
-#include "postmaster/pgarch.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -123,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/backend/meson.build b/src/backend/meson.build
index b1db3ba75b..4fdd209b82 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -7,6 +7,7 @@ backend_link_with = [pgport_srv, common_srv]
generated_backend_sources = []
subdir('access')
+subdir('archive')
subdir('backup')
subdir('bootstrap')
subdir('catalog')
diff --git a/src/backend/postmaster/Makefile b/src/backend/postmaster/Makefile
index 3a794e54d6..047448b34e 100644
--- a/src/backend/postmaster/Makefile
+++ b/src/backend/postmaster/Makefile
@@ -22,7 +22,6 @@ OBJS = \
interrupt.o \
pgarch.o \
postmaster.o \
- shell_archive.o \
startup.o \
syslogger.o \
walwriter.o
diff --git a/src/backend/postmaster/meson.build b/src/backend/postmaster/meson.build
index 9079922de7..cda921fd10 100644
--- a/src/backend/postmaster/meson.build
+++ b/src/backend/postmaster/meson.build
@@ -10,7 +10,6 @@ backend_sources += files(
'interrupt.c',
'pgarch.c',
'postmaster.c',
- 'shell_archive.c',
'startup.c',
'syslogger.c',
'walwriter.c',
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..46af349564 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -31,6 +31,8 @@
#include "access/xlog.h"
#include "access/xlog_internal.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "lib/binaryheap.h"
#include "libpq/pqsignal.h"
#include "pgstat.h"
@@ -97,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveContext;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -406,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveContext.check_configured_cb != NULL &&
- !ArchiveContext.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -508,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveContext.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -814,7 +817,7 @@ HandlePgArchInterrupts(void)
/*
* LoadArchiveLibrary
*
- * Loads the archiving callbacks into our local ArchiveContext.
+ * Loads the archiving callbacks into our local ArchiveCallbacks.
*/
static void
LoadArchiveLibrary(void)
@@ -827,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -844,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveContext);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveContext.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -859,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveContext.shutdown_cb != NULL)
- ArchiveContext.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 43b9d92660..1c0583fe26 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -33,6 +33,7 @@
#include "access/xlog_internal.h"
#include "access/xlogprefetcher.h"
#include "access/xlogrecovery.h"
+#include "archive/archive_module.h"
#include "catalog/namespace.h"
#include "catalog/storage.h"
#include "commands/async.h"
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
new file mode 100644
index 0000000000..92ced4b222
--- /dev/null
+++ b/src/include/archive/archive_module.h
@@ -0,0 +1,58 @@
+/*-------------------------------------------------------------------------
+ *
+ * archive_module.h
+ * Exports for archive modules.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/archive_module.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _ARCHIVE_MODULE_H
+#define _ARCHIVE_MODULE_H
+
+/*
+ * The value of the archive_library GUC.
+ */
+extern PGDLLIMPORT char *XLogArchiveLibrary;
+
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
+/*
+ * Archive module callbacks
+ *
+ * These callback functions should be defined by archive libraries and returned
+ * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
+ * For more information about the purpose of each callback, refer to the
+ * archive modules documentation.
+ */
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
+
+typedef struct ArchiveModuleCallbacks
+{
+ ArchiveStartupCB startup_cb;
+ ArchiveCheckConfiguredCB check_configured_cb;
+ ArchiveFileCB archive_file_cb;
+ ArchiveShutdownCB shutdown_cb;
+} ArchiveModuleCallbacks;
+
+/*
+ * Type of the shared library symbol _PG_archive_module_init that is looked
+ * up when loading an archive library.
+ */
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
+
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
+
+#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/archive/shell_archive.h b/src/include/archive/shell_archive.h
new file mode 100644
index 0000000000..9de6f769f1
--- /dev/null
+++ b/src/include/archive/shell_archive.h
@@ -0,0 +1,24 @@
+/*-------------------------------------------------------------------------
+ *
+ * shell_archive.h
+ * Exports for archiving via shell.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/shell_archive.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _SHELL_ARCHIVE_H
+#define _SHELL_ARCHIVE_H
+
+#include "archive/archive_module.h"
+
+/*
+ * Since the logic for archiving 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 const ArchiveModuleCallbacks *shell_archive_init(void);
+
+#endif /* _SHELL_ARCHIVE_H */
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index bcd51dfad6..3bd4fac71e 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -33,43 +33,4 @@ extern void PgArchiverMain(void) pg_attribute_noreturn();
extern void PgArchWakeup(void);
extern void PgArchForceDirScan(void);
-/*
- * The value of the archive_library GUC.
- */
-extern PGDLLIMPORT char *XLogArchiveLibrary;
-
-/*
- * Archive module callbacks
- *
- * These callback functions should be defined by archive libraries and returned
- * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
- * For more information about the purpose of each callback, refer to the
- * archive modules documentation.
- */
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
-
-typedef struct ArchiveModuleCallbacks
-{
- ArchiveCheckConfiguredCB check_configured_cb;
- ArchiveFileCB archive_file_cb;
- ArchiveShutdownCB shutdown_cb;
-} ArchiveModuleCallbacks;
-
-/*
- * Type of the shared library symbol _PG_archive_module_init that is looked
- * up when loading an archive library.
- */
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
-
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
-
-/*
- * Since the logic for archiving 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_archive_init(ArchiveModuleCallbacks *cb);
-
#endif /* _PGARCH_H */
--
2.25.1
--HcAYCG3uE/tztfnV--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v11 1/1] restructure archive modules API
@ 2023-02-09 21:49 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2023-02-09 21:49 UTC (permalink / raw)
---
contrib/basic_archive/basic_archive.c | 53 ++++++++++++-----
doc/src/sgml/archive-modules.sgml | 32 +++++++---
src/backend/Makefile | 2 +-
src/backend/archive/Makefile | 18 ++++++
src/backend/archive/meson.build | 5 ++
.../{postmaster => archive}/shell_archive.c | 30 ++++++----
src/backend/meson.build | 1 +
src/backend/postmaster/Makefile | 1 -
src/backend/postmaster/meson.build | 1 -
src/backend/postmaster/pgarch.c | 27 +++++----
src/backend/utils/misc/guc_tables.c | 1 +
src/include/archive/archive_module.h | 58 +++++++++++++++++++
src/include/archive/shell_archive.h | 22 +++++++
src/include/postmaster/pgarch.h | 39 -------------
14 files changed, 205 insertions(+), 85 deletions(-)
create mode 100644 src/backend/archive/Makefile
create mode 100644 src/backend/archive/meson.build
rename src/backend/{postmaster => archive}/shell_archive.c (78%)
create mode 100644 src/include/archive/archive_module.h
create mode 100644 src/include/archive/shell_archive.h
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 36b7a4814a..8a6bc25db5 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -30,9 +30,9 @@
#include <sys/time.h>
#include <unistd.h>
+#include "archive/archive_module.h"
#include "common/int.h"
#include "miscadmin.h"
-#include "postmaster/pgarch.h"
#include "storage/copydir.h"
#include "storage/fd.h"
#include "utils/guc.h"
@@ -40,15 +40,27 @@
PG_MODULE_MAGIC;
+typedef struct BasicArchiveData
+{
+ MemoryContext context;
+} BasicArchiveData;
+
static char *archive_directory = NULL;
-static MemoryContext basic_archive_context;
-static bool basic_archive_configured(void);
-static bool basic_archive_file(const char *file, const char *path);
+static void basic_archive_startup(ArchiveModuleState *state);
+static bool basic_archive_configured(ArchiveModuleState *state);
+static bool basic_archive_file(ArchiveModuleState *state, 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 const ArchiveModuleCallbacks basic_archive_callbacks = {
+ .startup_cb = basic_archive_startup,
+ .check_configured_cb = basic_archive_configured,
+ .archive_file_cb = basic_archive_file,
+ .shutdown_cb = NULL
+};
+
/*
* _PG_init
*
@@ -67,10 +79,6 @@ _PG_init(void)
check_archive_directory, NULL, NULL);
MarkGUCPrefixReserved("basic_archive");
-
- basic_archive_context = AllocSetContextCreate(TopMemoryContext,
- "basic_archive",
- ALLOCSET_DEFAULT_SIZES);
}
/*
@@ -78,11 +86,28 @@ _PG_init(void)
*
* Returns the module's archiving callbacks.
*/
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void)
+{
+ return &basic_archive_callbacks;
+}
+
+/*
+ * basic_archive_startup
+ *
+ * Creates the module's memory context.
+ */
void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+basic_archive_startup(ArchiveModuleState *state)
{
- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ BasicArchiveData *data;
+
+ data = (BasicArchiveData *) MemoryContextAllocZero(TopMemoryContext,
+ sizeof(BasicArchiveData));
+ data->context = AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+ state->private_data = (void *) data;
}
/*
@@ -133,7 +158,7 @@ check_archive_directory(char **newval, void **extra, GucSource source)
* Checks that archive_directory is not blank.
*/
static bool
-basic_archive_configured(void)
+basic_archive_configured(ArchiveModuleState *state)
{
return archive_directory != NULL && archive_directory[0] != '\0';
}
@@ -144,10 +169,12 @@ basic_archive_configured(void)
* Archives one file.
*/
static bool
-basic_archive_file(const char *file, const char *path)
+basic_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
sigjmp_buf local_sigjmp_buf;
MemoryContext oldcontext;
+ BasicArchiveData *data = (BasicArchiveData *) (state->private_data);
+ MemoryContext basic_archive_context = data->context;
/*
* We run basic_archive_file_internal() in our own memory context so that
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-modules.sgml
index ef02051f7f..8ea697a5bc 100644
--- a/doc/src/sgml/archive-modules.sgml
+++ b/doc/src/sgml/archive-modules.sgml
@@ -47,18 +47,22 @@
normal library search path is used to locate the library. To provide the
required archive module callbacks and to indicate that the library is
actually an archive module, it needs to provide a function named
- <function>_PG_archive_module_init</function>. This function is passed a
- struct that needs to be filled with the callback function pointers for
- individual actions.
+ <function>_PG_archive_module_init</function>. The result of the function
+ must be a pointer to a struct of type
+ <structname>ArchiveModuleCallbacks</structname>, which contains everything
+ that the core code needs to know how to make use of the archive module. The
+ return value needs to be of server lifetime, which is typically achieved by
+ defining it as a <literal>static const</literal> variable in global scope.
<programlisting>
typedef struct ArchiveModuleCallbacks
{
+ ArchiveStartupCB startup_cb;
ArchiveCheckConfiguredCB check_configured_cb;
ArchiveFileCB archive_file_cb;
ArchiveShutdownCB shutdown_cb;
} ArchiveModuleCallbacks;
-typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
</programlisting>
Only the <function>archive_file_cb</function> callback is required. The
@@ -73,6 +77,20 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
The server will call them as required to process each individual WAL file.
</para>
+ <sect2 id="archive-module-startup">
+ <title>Startup Callback</title>
+ <para>
+ The <function>startup_cb</function> callback is called shortly after the
+ module is loaded. This callback can be used to perform any additional
+ initialization required. If the archive module has state, it can use
+ <structfield>state->private_data</structfield> to store it.
+
+<programlisting>
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+</programlisting>
+ </para>
+ </sect2>
+
<sect2 id="archive-module-check">
<title>Check Callback</title>
<para>
@@ -83,7 +101,7 @@ typedef void (*ArchiveModuleInit) (struct ArchiveModuleCallbacks *cb);
assumes the module is configured.
<programlisting>
-typedef bool (*ArchiveCheckConfiguredCB) (void);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
</programlisting>
If <literal>true</literal> is returned, the server will proceed with
@@ -105,7 +123,7 @@ WARNING: archive_mode enabled, yet archiving is not configured
single WAL file.
<programlisting>
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
</programlisting>
If <literal>true</literal> is returned, the server proceeds as if the file
@@ -128,7 +146,7 @@ typedef bool (*ArchiveFileCB) (const char *file, const char *path);
these situations.
<programlisting>
-typedef void (*ArchiveShutdownCB) (void);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
</programlisting>
</para>
</sect2>
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 86e6dcb792..e4bf0fe9c0 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -17,7 +17,7 @@ subdir = src/backend
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = access backup bootstrap catalog parser commands executor \
+SUBDIRS = access archive backup bootstrap catalog parser commands executor \
foreign lib libpq \
main nodes optimizer partitioning port postmaster \
regex replication rewrite \
diff --git a/src/backend/archive/Makefile b/src/backend/archive/Makefile
new file mode 100644
index 0000000000..8d2860d0df
--- /dev/null
+++ b/src/backend/archive/Makefile
@@ -0,0 +1,18 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for src/backend/archive
+#
+# IDENTIFICATION
+# src/backend/archive/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/archive
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+OBJS = \
+ shell_archive.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/archive/meson.build b/src/backend/archive/meson.build
new file mode 100644
index 0000000000..e194282931
--- /dev/null
+++ b/src/backend/archive/meson.build
@@ -0,0 +1,5 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+backend_sources += files(
+ 'shell_archive.c'
+)
diff --git a/src/backend/postmaster/shell_archive.c b/src/backend/archive/shell_archive.c
similarity index 78%
rename from src/backend/postmaster/shell_archive.c
rename to src/backend/archive/shell_archive.c
index 7771b951b7..48e8952c0c 100644
--- a/src/backend/postmaster/shell_archive.c
+++ b/src/backend/archive/shell_archive.c
@@ -18,30 +18,36 @@
#include <sys/wait.h>
#include "access/xlog.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "common/percentrepl.h"
#include "pgstat.h"
-#include "postmaster/pgarch.h"
-static bool shell_archive_configured(void);
-static bool shell_archive_file(const char *file, const char *path);
-static void shell_archive_shutdown(void);
+static bool shell_archive_configured(ArchiveModuleState *state);
+static bool shell_archive_file(ArchiveModuleState *state, const char *file, const char *path);
+static void shell_archive_shutdown(ArchiveModuleState *state);
-void
-shell_archive_init(ArchiveModuleCallbacks *cb)
+static const ArchiveModuleCallbacks shell_archive_callbacks = {
+ .startup_cb = NULL,
+ .check_configured_cb = shell_archive_configured,
+ .archive_file_cb = shell_archive_file,
+ .shutdown_cb = shell_archive_shutdown
+};
+
+const ArchiveModuleCallbacks *
+shell_archive_init(void)
{
- cb->check_configured_cb = shell_archive_configured;
- cb->archive_file_cb = shell_archive_file;
- cb->shutdown_cb = shell_archive_shutdown;
+ return &shell_archive_callbacks;
}
static bool
-shell_archive_configured(void)
+shell_archive_configured(ArchiveModuleState *state)
{
return XLogArchiveCommand[0] != '\0';
}
static bool
-shell_archive_file(const char *file, const char *path)
+shell_archive_file(ArchiveModuleState *state, const char *file, const char *path)
{
char *xlogarchcmd;
char *nativePath = NULL;
@@ -123,7 +129,7 @@ shell_archive_file(const char *file, const char *path)
}
static void
-shell_archive_shutdown(void)
+shell_archive_shutdown(ArchiveModuleState *state)
{
elog(DEBUG1, "archiver process shutting down");
}
diff --git a/src/backend/meson.build b/src/backend/meson.build
index b1db3ba75b..4fdd209b82 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -7,6 +7,7 @@ backend_link_with = [pgport_srv, common_srv]
generated_backend_sources = []
subdir('access')
+subdir('archive')
subdir('backup')
subdir('bootstrap')
subdir('catalog')
diff --git a/src/backend/postmaster/Makefile b/src/backend/postmaster/Makefile
index 3a794e54d6..047448b34e 100644
--- a/src/backend/postmaster/Makefile
+++ b/src/backend/postmaster/Makefile
@@ -22,7 +22,6 @@ OBJS = \
interrupt.o \
pgarch.o \
postmaster.o \
- shell_archive.o \
startup.o \
syslogger.o \
walwriter.o
diff --git a/src/backend/postmaster/meson.build b/src/backend/postmaster/meson.build
index 9079922de7..cda921fd10 100644
--- a/src/backend/postmaster/meson.build
+++ b/src/backend/postmaster/meson.build
@@ -10,7 +10,6 @@ backend_sources += files(
'interrupt.c',
'pgarch.c',
'postmaster.c',
- 'shell_archive.c',
'startup.c',
'syslogger.c',
'walwriter.c',
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index e551af2905..46af349564 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -31,6 +31,8 @@
#include "access/xlog.h"
#include "access/xlog_internal.h"
+#include "archive/archive_module.h"
+#include "archive/shell_archive.h"
#include "lib/binaryheap.h"
#include "libpq/pqsignal.h"
#include "pgstat.h"
@@ -97,7 +99,8 @@ char *XLogArchiveLibrary = "";
*/
static time_t last_sigterm_time = 0;
static PgArchData *PgArch = NULL;
-static ArchiveModuleCallbacks ArchiveContext;
+static const ArchiveModuleCallbacks *ArchiveCallbacks;
+static ArchiveModuleState *archive_module_state;
/*
@@ -406,8 +409,8 @@ pgarch_ArchiverCopyLoop(void)
HandlePgArchInterrupts();
/* can't do anything if not configured ... */
- if (ArchiveContext.check_configured_cb != NULL &&
- !ArchiveContext.check_configured_cb())
+ if (ArchiveCallbacks->check_configured_cb != NULL &&
+ !ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
(errmsg("archive_mode enabled, yet archiving is not configured")));
@@ -508,7 +511,7 @@ pgarch_archiveXlog(char *xlog)
snprintf(activitymsg, sizeof(activitymsg), "archiving %s", xlog);
set_ps_display(activitymsg);
- ret = ArchiveContext.archive_file_cb(xlog, pathname);
+ ret = ArchiveCallbacks->archive_file_cb(archive_module_state, xlog, pathname);
if (ret)
snprintf(activitymsg, sizeof(activitymsg), "last was %s", xlog);
else
@@ -814,7 +817,7 @@ HandlePgArchInterrupts(void)
/*
* LoadArchiveLibrary
*
- * Loads the archiving callbacks into our local ArchiveContext.
+ * Loads the archiving callbacks into our local ArchiveCallbacks.
*/
static void
LoadArchiveLibrary(void)
@@ -827,8 +830,6 @@ LoadArchiveLibrary(void)
errmsg("both archive_command and archive_library set"),
errdetail("Only one of archive_command, archive_library may be set.")));
- memset(&ArchiveContext, 0, sizeof(ArchiveModuleCallbacks));
-
/*
* If shell archiving is enabled, use our special initialization function.
* Otherwise, load the library and call its _PG_archive_module_init().
@@ -844,12 +845,16 @@ LoadArchiveLibrary(void)
ereport(ERROR,
(errmsg("archive modules have to define the symbol %s", "_PG_archive_module_init")));
- (*archive_init) (&ArchiveContext);
+ ArchiveCallbacks = (*archive_init) ();
- if (ArchiveContext.archive_file_cb == NULL)
+ if (ArchiveCallbacks->archive_file_cb == NULL)
ereport(ERROR,
(errmsg("archive modules must register an archive callback")));
+ archive_module_state = (ArchiveModuleState *) palloc0(sizeof(ArchiveModuleState));
+ if (ArchiveCallbacks->startup_cb != NULL)
+ ArchiveCallbacks->startup_cb(archive_module_state);
+
before_shmem_exit(pgarch_call_module_shutdown_cb, 0);
}
@@ -859,6 +864,6 @@ LoadArchiveLibrary(void)
static void
pgarch_call_module_shutdown_cb(int code, Datum arg)
{
- if (ArchiveContext.shutdown_cb != NULL)
- ArchiveContext.shutdown_cb();
+ if (ArchiveCallbacks->shutdown_cb != NULL)
+ ArchiveCallbacks->shutdown_cb(archive_module_state);
}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b46e3b8c55..91cb64602b 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -33,6 +33,7 @@
#include "access/xlog_internal.h"
#include "access/xlogprefetcher.h"
#include "access/xlogrecovery.h"
+#include "archive/archive_module.h"
#include "catalog/namespace.h"
#include "catalog/storage.h"
#include "commands/async.h"
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
new file mode 100644
index 0000000000..92ced4b222
--- /dev/null
+++ b/src/include/archive/archive_module.h
@@ -0,0 +1,58 @@
+/*-------------------------------------------------------------------------
+ *
+ * archive_module.h
+ * Exports for archive modules.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/archive_module.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _ARCHIVE_MODULE_H
+#define _ARCHIVE_MODULE_H
+
+/*
+ * The value of the archive_library GUC.
+ */
+extern PGDLLIMPORT char *XLogArchiveLibrary;
+
+typedef struct ArchiveModuleState
+{
+ /*
+ * Private data pointer for use by an archive module. This can be used to
+ * store state for the module that will be passed to each of its callbacks.
+ */
+ void *private_data;
+} ArchiveModuleState;
+
+/*
+ * Archive module callbacks
+ *
+ * These callback functions should be defined by archive libraries and returned
+ * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
+ * For more information about the purpose of each callback, refer to the
+ * archive modules documentation.
+ */
+typedef void (*ArchiveStartupCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveCheckConfiguredCB) (ArchiveModuleState *state);
+typedef bool (*ArchiveFileCB) (ArchiveModuleState *state, const char *file, const char *path);
+typedef void (*ArchiveShutdownCB) (ArchiveModuleState *state);
+
+typedef struct ArchiveModuleCallbacks
+{
+ ArchiveStartupCB startup_cb;
+ ArchiveCheckConfiguredCB check_configured_cb;
+ ArchiveFileCB archive_file_cb;
+ ArchiveShutdownCB shutdown_cb;
+} ArchiveModuleCallbacks;
+
+/*
+ * Type of the shared library symbol _PG_archive_module_init that is looked
+ * up when loading an archive library.
+ */
+typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
+
+extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
+
+#endif /* _ARCHIVE_MODULE_H */
diff --git a/src/include/archive/shell_archive.h b/src/include/archive/shell_archive.h
new file mode 100644
index 0000000000..1c843c13c5
--- /dev/null
+++ b/src/include/archive/shell_archive.h
@@ -0,0 +1,22 @@
+/*-------------------------------------------------------------------------
+ *
+ * shell_archive.h
+ * Exports for archiving via shell.
+ *
+ * Copyright (c) 2022-2023, PostgreSQL Global Development Group
+ *
+ * src/include/archive/shell_archive.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _SHELL_ARCHIVE_H
+#define _SHELL_ARCHIVE_H
+
+/*
+ * Since the logic for archiving 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 const ArchiveModuleCallbacks *shell_archive_init(void);
+
+#endif /* _SHELL_ARCHIVE_H */
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index bcd51dfad6..3bd4fac71e 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -33,43 +33,4 @@ extern void PgArchiverMain(void) pg_attribute_noreturn();
extern void PgArchWakeup(void);
extern void PgArchForceDirScan(void);
-/*
- * The value of the archive_library GUC.
- */
-extern PGDLLIMPORT char *XLogArchiveLibrary;
-
-/*
- * Archive module callbacks
- *
- * These callback functions should be defined by archive libraries and returned
- * via _PG_archive_module_init(). ArchiveFileCB is the only required callback.
- * For more information about the purpose of each callback, refer to the
- * archive modules documentation.
- */
-typedef bool (*ArchiveCheckConfiguredCB) (void);
-typedef bool (*ArchiveFileCB) (const char *file, const char *path);
-typedef void (*ArchiveShutdownCB) (void);
-
-typedef struct ArchiveModuleCallbacks
-{
- ArchiveCheckConfiguredCB check_configured_cb;
- ArchiveFileCB archive_file_cb;
- ArchiveShutdownCB shutdown_cb;
-} ArchiveModuleCallbacks;
-
-/*
- * Type of the shared library symbol _PG_archive_module_init that is looked
- * up when loading an archive library.
- */
-typedef void (*ArchiveModuleInit) (ArchiveModuleCallbacks *cb);
-
-extern PGDLLEXPORT void _PG_archive_module_init(ArchiveModuleCallbacks *cb);
-
-/*
- * Since the logic for archiving 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_archive_init(ArchiveModuleCallbacks *cb);
-
#endif /* _PGARCH_H */
--
2.25.1
--k+w/mQv8wyuph6w0--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v47 8/9] Error out any process that would block at REPACK
@ 2026-03-25 19:35 Álvaro Herrera <alvherre@kurilemu.de>
0 siblings, 0 replies; 15+ messages in thread
From: Álvaro Herrera @ 2026-03-25 19:35 UTC (permalink / raw)
Any process waiting on REPACK to release its lock would actually cause
it to deadlock when it tries to upgrade its lock to AEL, losing all work
done to that point. We avoid this by teaching the deadlock detector to
raise an error when this condition is detected.
---
src/backend/commands/repack.c | 52 ++++++++----
src/backend/storage/lmgr/deadlock.c | 15 ++++
src/include/storage/proc.h | 6 +-
src/test/modules/injection_points/Makefile | 1 +
.../expected/repack_deadlock.out | 63 ++++++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../specs/repack_deadlock.spec | 83 +++++++++++++++++++
7 files changed, 202 insertions(+), 19 deletions(-)
create mode 100644 src/test/modules/injection_points/expected/repack_deadlock.out
create mode 100644 src/test/modules/injection_points/specs/repack_deadlock.spec
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 9c807f75d71..3acb2e1d2ba 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -276,6 +276,16 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
/* Determine the lock mode to use. */
lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0);
+ /*
+ * If in concurrent mode, set the PROC_IN_CONCURRENT_REPACK flag. This
+ * makes the deadlock checker cause anyone that would conflict with us
+ * to error out. It's important to set this flag ahead of actually locking
+ * the relation; it won't of course affect anyone until we do have a lock
+ * that others can conflict with.
+ */
+ if ((params.options & CLUOPT_CONCURRENT) != 0)
+ MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
+
/*
* If a single relation is specified, process it and we're done ... unless
* the relation is a partitioned table, in which case we fall through.
@@ -476,11 +486,8 @@ RepackLockLevel(bool concurrent)
* If indexOid is InvalidOid, the table will be rewritten in physical order
* instead of index order.
*
- * Note that, in the concurrent case, the function releases the lock at some
- * point, in order to get AccessExclusiveLock for the final steps (i.e. to
- * swap the relation files). To make things simpler, the caller should expect
- * OldHeap to be closed on return, regardless CLUOPT_CONCURRENT. (The
- * AccessExclusiveLock is kept till the end of the transaction.)
+ * On return, OldHeap is closed but locked with AccessExclusiveLock - the lock
+ * will be released at end of the transaction.
*
* 'cmd' indicates which command is being executed, to be used for error
* messages.
@@ -512,10 +519,12 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
/*
* Make sure we're not in a transaction block.
*
- * The reason is that repack_setup_logical_decoding() could deadlock
- * if there's an XID already assigned. It would be possible to run in
- * a transaction block if we had no XID, but this restriction is
- * simpler for users to understand and we don't lose anything.
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) It would be possible to run in a transaction
+ * block if we had no XID, but this restriction is simpler for users
+ * to understand and we don't lose anything.
*/
PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
@@ -998,10 +1007,8 @@ rebuild_relation(Relation OldHeap, Relation index, bool verbose,
* Note that the worker has to wait for all transactions with XID
* already assigned to finish. If some of those transactions is
* waiting for a lock conflicting with ShareUpdateExclusiveLock on our
- * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
- * Not sure this risk is worth unlocking/locking the table (and its
- * clustering index) and checking again if it's still eligible for
- * REPACK CONCURRENTLY.
+ * table (e.g. it runs CREATE INDEX), it should encounter ERROR in the
+ * deadlock checking code.
*/
start_repack_decoding_worker(tableOid);
@@ -3090,7 +3097,16 @@ rebuild_relation_finish_concurrent(Relation NewHeap, Relation OldHeap,
LockRelationOid(OldHeap->rd_rel->reltoastrelid, AccessExclusiveLock);
/*
- * Tuples and pages of the old heap will be gone, but the heap will stay.
+ * Now that we have all access-exclusive locks on all relations, we no
+ * longer want other processes to error out when trying to acquire a
+ * conflicting lock. Therefore, unset our flag.
+ */
+ MyProc->statusFlags &= ~PROC_IN_CONCURRENT_REPACK;
+
+ /*
+ * Tuples and pages of the old heap will be gone, but the heap itself will
+ * stay. In order for predicate locks to continue to work, convert them
+ * to relation-level locks. We do this both for table and indexes.
*/
TransferPredicateLocksToHeapRelation(OldHeap);
foreach_ptr(RelationData, index, indexrels)
@@ -3287,9 +3303,11 @@ start_repack_decoding_worker(Oid relid)
/*
* The decoding setup must be done before the caller can have XID assigned
- * for any reason, otherwise the worker might end up in a deadlock,
- * waiting for the caller's transaction to end. Therefore wait here until
- * the worker indicates that it has the logical decoding initialized.
+ * for any reason, otherwise the worker might end up waiting for the
+ * caller's transaction to end. (Deadlock detector does not consider this
+ * a conflict because the worker is in the same locking group as the
+ * backend that launched it.) Therefore wait here until the worker
+ * indicates that it has the logical decoding initialized.
*/
ConditionVariablePrepareToSleep(&shared->cv);
for (;;)
diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c
index b8962d875b6..3d3ec0da111 100644
--- a/src/backend/storage/lmgr/deadlock.c
+++ b/src/backend/storage/lmgr/deadlock.c
@@ -620,6 +620,21 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
proc->statusFlags & PROC_IS_AUTOVACUUM)
blocking_autovacuum_proc = proc;
+ /*
+ * Similarly, if we note that we're blocked by some process
+ * running REPACK (CONCURRENTLY), just fail. That process
+ * is going to upgrade its lock at some point, and it would
+ * be inappropriate for any other process to cause that
+ * to fail.
+ */
+ if (checkProc == MyProc &&
+ proc->statusFlags & PROC_IN_CONCURRENT_REPACK)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_IN_USE),
+ errmsg("could not wait for concurrent REPACK"),
+ errdetail("Process %d waits for REPACK running on process %d",
+ MyProc->pid, proc->pid));
+
/* We're done looking at this proclock */
break;
}
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 1dad125706e..8ad9718f3d6 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -69,10 +69,12 @@ struct XidCache
#define PROC_AFFECTS_ALL_HORIZONS 0x20 /* this proc's xmin must be
* included in vacuum horizons
* in all databases */
+#define PROC_IN_CONCURRENT_REPACK 0x40 /* REPACK (CONCURRENTLY) */
-/* flags reset at EOXact */
+/* flags reset at EOXact. A bit of a misnomer ... */
#define PROC_VACUUM_STATE_MASK \
- (PROC_IN_VACUUM | PROC_IN_SAFE_IC | PROC_VACUUM_FOR_WRAPAROUND)
+ (PROC_IN_VACUUM | PROC_IN_SAFE_IC | PROC_VACUUM_FOR_WRAPAROUND | \
+ PROC_IN_CONCURRENT_REPACK)
/*
* Xmin-related flags. Make sure any flags that affect how the process' Xmin
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 2cd7d87c533..f7663859fe2 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -15,6 +15,7 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
ISOLATION = basic \
inplace \
repack \
+ repack_deadlock \
repack_toast \
syscache-update-pruned \
heap_lock_update
diff --git a/src/test/modules/injection_points/expected/repack_deadlock.out b/src/test/modules/injection_points/expected/repack_deadlock.out
new file mode 100644
index 00000000000..a86e4767536
--- /dev/null
+++ b/src/test/modules/injection_points/expected/repack_deadlock.out
@@ -0,0 +1,63 @@
+Parsed test spec with 2 sessions
+
+starting permutation: wait_before_lock add_column wakeup_before_lock check1
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step wait_before_lock:
+ REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey;
+ <waiting ...>
+step add_column:
+ alter table repack_deadlock add column noise text;
+ <waiting ...>
+step add_column: <... completed>
+ERROR: could not wait for concurrent REPACK
+step wakeup_before_lock:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step wait_before_lock: <... completed>
+step check1:
+ INSERT INTO relfilenodes(node)
+ SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock';
+
+ SELECT count(DISTINCT node) FROM relfilenodes;
+
+ SELECT i, j FROM repack_deadlock ORDER BY i, j;
+
+ INSERT INTO data_s1(i, j)
+ SELECT i, j FROM repack_deadlock;
+
+ SELECT count(*)
+ FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j)
+ WHERE d1.i ISNULL OR d2.i ISNULL;
+
+count
+-----
+ 1
+(1 row)
+
+i|j
+-+-
+1|1
+2|2
+3|3
+4|4
+(4 rows)
+
+count
+-----
+ 4
+(1 row)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index a414abb924b..1cd88d6db65 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -46,6 +46,7 @@ tests += {
'basic',
'inplace',
'repack',
+ 'repack_deadlock',
'repack_toast',
'syscache-update-pruned',
'heap_lock_update',
diff --git a/src/test/modules/injection_points/specs/repack_deadlock.spec b/src/test/modules/injection_points/specs/repack_deadlock.spec
new file mode 100644
index 00000000000..9d23a6588c2
--- /dev/null
+++ b/src/test/modules/injection_points/specs/repack_deadlock.spec
@@ -0,0 +1,83 @@
+# Test REPACK with a concurrent transaction that would cause a deadlock
+setup
+{
+ CREATE EXTENSION injection_points;
+
+ CREATE TABLE repack_deadlock(i int PRIMARY KEY, j int);
+ INSERT INTO repack_deadlock(i, j) VALUES (1, 1), (2, 2), (3, 3), (4, 4);
+
+ CREATE TABLE relfilenodes(node oid);
+
+ CREATE TABLE data_s1(i int, j int);
+ CREATE TABLE data_s2(i int, j int);
+}
+
+teardown
+{
+ DROP TABLE repack_deadlock;
+ DROP EXTENSION injection_points;
+
+ DROP TABLE relfilenodes;
+ DROP TABLE data_s1;
+ DROP TABLE data_s2;
+}
+
+session s1
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
+}
+# Perform the initial load and wait for s2 to do some data changes.
+step wait_before_lock
+{
+ REPACK (CONCURRENTLY) repack_deadlock USING INDEX repack_deadlock_pkey;
+}
+# Check the table from the perspective of s1.
+#
+# Besides the contents, we also check that relfilenode has changed.
+
+# Have each session write the contents into a table and use FULL JOIN to check
+# if the outputs are identical.
+step check1
+{
+ INSERT INTO relfilenodes(node)
+ SELECT relfilenode FROM pg_class WHERE relname='repack_deadlock';
+
+ SELECT count(DISTINCT node) FROM relfilenodes;
+
+ SELECT i, j FROM repack_deadlock ORDER BY i, j;
+
+ INSERT INTO data_s1(i, j)
+ SELECT i, j FROM repack_deadlock;
+
+ SELECT count(*)
+ FROM data_s1 d1 FULL JOIN data_s2 d2 USING (i, j)
+ WHERE d1.i ISNULL OR d2.i ISNULL;
+}
+teardown
+{
+ SELECT injection_points_detach('repack-concurrently-before-lock');
+}
+
+session s2
+# Change the existing data. UPDATE changes both key and non-key columns. Also
+# update one row twice to test whether tuple version generated by this session
+# can be found.
+step add_column
+{
+ alter table repack_deadlock add column noise text;
+}
+
+step wakeup_before_lock
+{
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+}
+
+# Test if data changes introduced while one session is performing REPACK
+# CONCURRENTLY find their way into the table.
+permutation
+ wait_before_lock
+ add_column
+ wakeup_before_lock
+ check1
--
2.47.3
--2fxmamo6mu2qbxgv
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v47-0009-Teach-snapshot-builder-to-skip-transactions-runn.patch"
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2026-03-25 19:35 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-01-31 22:58 [PATCH v9 3/3] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-01-31 22:58 [PATCH v5 3/3] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-01-31 22:58 [PATCH v4 3/3] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-01-31 22:58 [PATCH v2 3/3] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-01-31 22:58 [PATCH v3 3/3] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-01-31 22:58 [PATCH v6 3/3] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-01-31 22:58 [PATCH v8 4/4] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-01-31 22:58 [PATCH v7 3/4] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-02-09 21:49 [PATCH v12 1/1] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-02-09 21:49 [PATCH v14 1/1] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-02-09 21:49 [PATCH v10 1/1] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-02-09 21:49 [PATCH v13 1/1] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-02-09 21:49 [PATCH v15 1/1] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2023-02-09 21:49 [PATCH v11 1/1] restructure archive modules API Nathan Bossart <nathandbossart@gmail.com>
2026-03-25 19:35 [PATCH v47 8/9] Error out any process that would block at REPACK Álvaro Herrera <alvherre@kurilemu.de>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox