v10-0002-Add-error-callbacks-for-deallocating-backup-rela.patch

From 9e685c75676c763ade9c0b2d938d1b2e567e2c93 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Wed, 21 Sep 2022 11:02:53 +0000
Subject: [PATCH v10] Add error callbacks for deallocating backup related
 variables

This adds error callbacks for deallocating backup related
variables to avoid memory bloat and save on some 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 a9addf6e0c..83724548ef 100644
--- a/src/backend/access/transam/xlogbackup.c
+++ b/src/backend/access/transam/xlogbackup.c
@@ -94,3 +94,25 @@ deallocate_backup_variables(BackupState *state, StringInfo backup_label,
 	if (tablespace_map != NULL)
 		pfree(tablespace_map->data);
 }
+
+/*
+ * Error 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);
+}
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 8b06d977b9..49a88a9068 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);
 
@@ -102,9 +130,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(backupidstr, fast, NULL, backup_state, tablespace_map);
 
+	/* Pop the error context stack */
+	error_context_stack = errcallback.previous;
+
 	PG_RETURN_LSN(backup_state->startpoint);
 }
 
@@ -138,6 +178,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)
@@ -156,6 +198,15 @@ 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.
 	 */
@@ -169,6 +220,9 @@ pg_backup_stop(PG_FUNCTION_ARGS)
 	backup_label = makeStringInfo();
 	build_backup_content(backup_state, false, backup_label);
 
+	/* Pop the error context stack */
+	error_context_stack = errcallback.previous;
+
 	values[0] = LSNGetDatum(backup_state->stoppoint);
 	values[1] = CStringGetTextDatum(backup_label->data);
 	values[2] = CStringGetTextDatum(tablespace_map->data);
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index fb3b59832b..d20aa20950 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(opt->label, opt->fastcheckpoint, &state.tablespaces,
 					   backup_state, tablespace_map);
@@ -667,6 +678,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 eb724a9d7f..6519fb310c 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 69b960c417..4c80070ce6 100644
--- a/src/include/access/xlogbackup.h
+++ b/src/include/access/xlogbackup.h
@@ -38,11 +38,22 @@ 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 void build_backup_content(BackupState *state,
 								 bool ishistoryfile,
 								 StringInfo str);
 extern void deallocate_backup_variables(BackupState *state,
 										StringInfo backup_label,
 										StringInfo tablespace_map);
+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