1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-30 11:03:19 +03:00

Add built-in ERROR handling for archive callbacks.

Presently, the archiver process restarts when an archive callback
ERRORs.  To avoid this, archive module authors can use sigsetjmp(),
manage a memory context, etc., but that requires a lot of extra
code that will likely look roughly the same between modules.  This
commit adds basic archive callback ERROR handling to pgarch.c so
that module authors won't ordinarily need to worry about this.
While this built-in handler attempts to clean up anything that an
archive module could conceivably have left behind, it is possible
that some modules are doing unexpected things that require
additional cleanup.  Module authors should be sure to do any extra
required cleanup in a PG_CATCH block within the archiving callback.

The archiving callback is now called in a short-lived memory
context that the archiver process resets between invocations.  If a
module requires longer-lived storage, it must maintain its own
memory context.

Thanks to these changes, the basic_archive module can be greatly
simplified.

Suggested-by: Andres Freund
Reviewed-by: Andres Freund, Yong Li
Discussion: https://postgr.es/m/20230217215624.GA3131134%40nathanxps13
This commit is contained in:
Nathan Bossart
2024-04-02 22:28:11 -05:00
parent 5bec1d6bc5
commit c627d944e6
4 changed files with 107 additions and 136 deletions

View File

@ -40,26 +40,18 @@
PG_MODULE_MAGIC;
typedef struct BasicArchiveData
{
MemoryContext context;
} BasicArchiveData;
static char *archive_directory = NULL;
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,
.startup_cb = NULL,
.check_configured_cb = basic_archive_configured,
.archive_file_cb = basic_archive_file,
.shutdown_cb = basic_archive_shutdown
.shutdown_cb = NULL
};
/*
@ -93,24 +85,6 @@ _PG_archive_module_init(void)
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;
}
/*
* check_archive_directory
*
@ -176,74 +150,6 @@ basic_archive_configured(ArchiveModuleState *state)
*/
static bool
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
* we can easily reset it during error recovery (thus avoiding memory
* leaks).
*/
oldcontext = MemoryContextSwitchTo(basic_archive_context);
/*
* Since the archiver operates at the bottom of the exception stack,
* ERRORs turn into FATALs and cause the archiver process to restart.
* However, using ereport(ERROR, ...) when there are problems is easy to
* code and maintain. Therefore, we create our own exception handler to
* catch ERRORs and return false instead of restarting the archiver
* whenever there is a failure.
*/
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
{
/* Since not using PG_TRY, must reset error stack by hand */
error_context_stack = NULL;
/* Prevent interrupts while cleaning up */
HOLD_INTERRUPTS();
/* Report the error and clear ErrorContext for next time */
EmitErrorReport();
FlushErrorState();
/* Close any files left open by copy_file() or compare_files() */
AtEOSubXact_Files(false, InvalidSubTransactionId, InvalidSubTransactionId);
/* Reset our memory context and switch back to the original one */
MemoryContextSwitchTo(oldcontext);
MemoryContextReset(basic_archive_context);
/* Remove our exception handler */
PG_exception_stack = NULL;
/* Now we can allow interrupts again */
RESUME_INTERRUPTS();
/* Report failure so that the archiver retries this file */
return false;
}
/* Enable our exception handler */
PG_exception_stack = &local_sigjmp_buf;
/* Archive the file! */
basic_archive_file_internal(file, path);
/* Remove our exception handler */
PG_exception_stack = NULL;
/* Reset our memory context and switch back to the original one */
MemoryContextSwitchTo(oldcontext);
MemoryContextReset(basic_archive_context);
return true;
}
static void
basic_archive_file_internal(const char *file, const char *path)
{
char destination[MAXPGPATH];
char temp[MAXPGPATH + 256];
@ -277,7 +183,7 @@ basic_archive_file_internal(const char *file, const char *path)
fsync_fname(destination, false);
fsync_fname(archive_directory, true);
return;
return true;
}
ereport(ERROR,
@ -317,6 +223,8 @@ basic_archive_file_internal(const char *file, const char *path)
ereport(DEBUG1,
(errmsg("archived \"%s\" via basic_archive", file)));
return true;
}
/*
@ -399,35 +307,3 @@ 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;
}