From b20d5455f746ec8f2ca4bd325f8636aa2b991438 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Tue, 20 Sep 2022 10:44:42 +0000 Subject: [PATCH v9] Add error callbacks for deallocating backup related variables This adds error callbacks for deallocating backup related variables to save on some palloc-ed memory upon errors while taking backup. --- src/backend/access/transam/xlogbackup.c | 22 ++++++++++ src/backend/access/transam/xlogfuncs.c | 54 +++++++++++++++++++++++++ src/backend/backup/basebackup.c | 14 +++++++ src/backend/utils/error/elog.c | 17 ++++++++ src/include/access/xlogbackup.h | 11 +++++ src/include/utils/elog.h | 1 + 6 files changed, 119 insertions(+) diff --git a/src/backend/access/transam/xlogbackup.c b/src/backend/access/transam/xlogbackup.c index 38c37cc7db..39210af41b 100644 --- a/src/backend/access/transam/xlogbackup.c +++ b/src/backend/access/transam/xlogbackup.c @@ -92,6 +92,28 @@ deallocate_backup_variables(BackupState *state, StringInfo backup_label, pfree(tablespace_map->data); } +/* + * Erro callback for deallocating backup variables. + */ +void +deallocate_backup_variables_err_cb(void *arg) +{ + BackupErrorInfo *errinfo = (BackupErrorInfo *) arg; + + /* + * Since error callbacks are called for elevel lesser than ERROR, we + * proceed further only if elevel is ERROR or greater. + */ + if (geterrlevel() < ERROR) + return; + + if (errinfo == NULL) + return; /* nothing to do */ + + deallocate_backup_variables(errinfo->state, errinfo->backup_label, + errinfo->tablespace_map); +} + /* * Construct backup_label file or history file contents * diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index e078579e6f..7d801aefc7 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -45,6 +45,32 @@ static BackupState *backup_state = NULL; static StringInfo tablespace_map = NULL; +/* + * Erro callback for deallocating and resetting backup variables. + */ +static void +deallocate_and_reset_backup_variables_err_cb(void *arg) +{ + BackupErrorInfo *errinfo = (BackupErrorInfo *) arg; + + /* + * Since error callbacks are called for elevel lesser than ERROR, we + * proceed further only if elevel is ERROR or greater. + */ + if (geterrlevel() < ERROR) + return; + + if (errinfo == NULL) + return; /* nothing to do */ + + deallocate_backup_variables(errinfo->state, errinfo->backup_label, + errinfo->tablespace_map); + + /* Reset the static variables. */ + backup_state = NULL; + tablespace_map = NULL; +} + /* * pg_backup_start: start taking an on-line backup. * @@ -61,6 +87,8 @@ pg_backup_start(PG_FUNCTION_ARGS) char *backupidstr; SessionBackupState status = get_backup_status(); MemoryContext oldcontext; + ErrorContextCallback errcallback; + BackupErrorInfo errinfo; backupidstr = text_to_cstring(backupid); @@ -93,9 +121,21 @@ pg_backup_start(PG_FUNCTION_ARGS) tablespace_map = makeStringInfo(); MemoryContextSwitchTo(oldcontext); + /* Set up callback to clean up backup related variables */ + errcallback.callback = deallocate_and_reset_backup_variables_err_cb; + errinfo.state = backup_state; + errinfo.backup_label = NULL; + errinfo.tablespace_map = tablespace_map; + errcallback.arg = (void *) &errinfo; + errcallback.previous = error_context_stack; + error_context_stack = &errcallback; + register_persistent_abort_backup_handler(); do_pg_backup_start(backup_state, fast, NULL, tablespace_map); + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + PG_RETURN_LSN(backup_state->startpoint); } @@ -123,6 +163,8 @@ pg_backup_stop(PG_FUNCTION_ARGS) bool waitforarchive = PG_GETARG_BOOL(0); StringInfo backup_label; SessionBackupState status = get_backup_status(); + ErrorContextCallback errcallback; + BackupErrorInfo errinfo; /* Initialize attributes information in the tuple descriptor */ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) @@ -141,12 +183,24 @@ pg_backup_stop(PG_FUNCTION_ARGS) Assert(backup_state != NULL); Assert(tablespace_map != NULL); + /* Set up callback to clean up backup related variables */ + errcallback.callback = deallocate_and_reset_backup_variables_err_cb; + errinfo.state = backup_state; + errinfo.backup_label = NULL; + errinfo.tablespace_map = tablespace_map; + errcallback.arg = (void *) &errinfo; + errcallback.previous = error_context_stack; + error_context_stack = &errcallback; + /* * Stop the backup. Return a copy of the backup label and tablespace map * so they can be written to disk by the caller. */ do_pg_backup_stop(backup_state, waitforarchive); + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + /* * Construct backup_label contents. Note that the backup_label doesn't have * to be in TopMemoryContext unlike backup_state and tablespace_map as it diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index 7ba4f340bc..3edbe4c460 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -236,6 +236,8 @@ perform_base_backup(basebackup_options *opt, bbsink *sink) BackupState *backup_state; StringInfo backup_label; StringInfo tablespace_map; + ErrorContextCallback errcallback; + BackupErrorInfo errinfo; /* Initial backup state, insofar as we know it now. */ state.tablespaces = NIL; @@ -260,6 +262,15 @@ perform_base_backup(basebackup_options *opt, bbsink *sink) backup_label = makeStringInfo(); tablespace_map = makeStringInfo(); + /* Set up callback to clean up backup related variables */ + errcallback.callback = deallocate_backup_variables_err_cb; + errinfo.state = backup_state; + errinfo.backup_label = backup_label; + errinfo.tablespace_map = tablespace_map; + errcallback.arg = (void *) &errinfo; + errcallback.previous = error_context_stack; + error_context_stack = &errcallback; + basebackup_progress_wait_checkpoint(); do_pg_backup_start(backup_state, opt->fastcheckpoint, &state.tablespaces, tablespace_map); @@ -663,6 +674,9 @@ perform_base_backup(basebackup_options *opt, bbsink *sink) /* clean up the resource owner we created */ WalSndResourceCleanup(true); + /* Pop the error context stack */ + error_context_stack = errcallback.previous; + basebackup_progress_done(); } diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 96c694da8f..d2868648db 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -1404,6 +1404,23 @@ geterrcode(void) return edata->sqlerrcode; } +/* + * geterrlevel --- return the currently error level + * + * This is only intended for use in error callback subroutines, since there + * is no other place outside elog.c where the concept is meaningful. + */ +int +geterrlevel(void) +{ + ErrorData *edata = &errordata[errordata_stack_depth]; + + /* we don't bother incrementing recursion_depth */ + CHECK_STACK_DEPTH(); + + return edata->elevel; +} + /* * geterrposition --- return the currently set error position (0 if none) * diff --git a/src/include/access/xlogbackup.h b/src/include/access/xlogbackup.h index 5b43b7d1d7..e98fe6132c 100644 --- a/src/include/access/xlogbackup.h +++ b/src/include/access/xlogbackup.h @@ -42,6 +42,16 @@ typedef struct BackupState pg_time_t stoptime; /* backup stop time */ } BackupState; +/* + * Structure to hold input parameters for backup error callback. + */ +typedef struct BackupErrorInfo +{ + BackupState *state; + StringInfo backup_label; + StringInfo tablespace_map; +} BackupErrorInfo; + extern BackupState *allocate_backup_state(const char *name); extern void reset_backup_state(BackupState *state, const char *name); extern void deallocate_backup_variables(BackupState *state, @@ -50,4 +60,5 @@ extern void deallocate_backup_variables(BackupState *state, extern void build_backup_content(BackupState *state, bool ishistoryfile, StringInfo str); +extern void deallocate_backup_variables_err_cb(void *arg); #endif /* XLOG_BACKUP_H */ diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index 4dd9658a3c..a8955f9572 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -222,6 +222,7 @@ extern int internalerrquery(const char *query); extern int err_generic_string(int field, const char *str); extern int geterrcode(void); +extern int geterrlevel(void); extern int geterrposition(void); extern int getinternalerrposition(void); -- 2.34.1