mirror of
https://git.postgresql.org/git/postgresql.git
synced 2025-01-24 18:55:04 +08:00
Improve memory handling across SQL-callable backup functions
Since pg_backup_start() and pg_backup_stop() exist, the tablespace map
data and the backup state data (backup_label string until 7d70809
) have
been allocated in the TopMemoryContext. This approach would cause
memory leaks in the session calling these functions if failures happen
before pg_backup_stop() ends, leaking more memory on repeated failures.
Both things need little memory so that would not be really noticeable
for most users, except perhaps connection poolers with long-lived
connections able to trigger backup failures with these functions.
This commit improves the logic in this area by not allocating anymore
the backup-related data that needs to travel across the SQL-callable
backup functions in TopMemoryContext, by using instead a dedicated
memory context child of TopMemoryContext. The memory context is created
in pg_backup_start() and deleted when finishing pg_backup_stop(). In
the event of an in-flight failure, this memory context gets reset in the
follow-up pg_backup_start() call, so as we are sure that only one run
worth of data is leaked at any time. Some cleanup was already done for
the backup data on a follow-up call of pg_backup_start(), but using a
memory context makes the whole simpler.
BASE_BACKUP commands are executed in isolation, relying on the memory
context created for replication commands, hence these do not need such
an extra logic.
Author: Bharath Rupireddy
Reviewed-by: Robert Haas, Alvaro Herrera, Cary Huang, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACXqvfKF2B0beQ=aJMdWnpNohmBPsRg=EDQj_6y1t2O8mQ@mail.gmail.com
This commit is contained in:
parent
1f0c4fa255
commit
6cc66197ff
@ -45,6 +45,9 @@
|
||||
static BackupState *backup_state = NULL;
|
||||
static StringInfo tablespace_map = NULL;
|
||||
|
||||
/* Session-level context for the SQL-callable backup functions */
|
||||
static MemoryContext backupcontext = NULL;
|
||||
|
||||
/*
|
||||
* pg_backup_start: set up for taking an on-line backup dump
|
||||
*
|
||||
@ -72,27 +75,26 @@ pg_backup_start(PG_FUNCTION_ARGS)
|
||||
|
||||
/*
|
||||
* backup_state and tablespace_map need to be long-lived as they are used
|
||||
* in pg_backup_stop().
|
||||
* in pg_backup_stop(). These are allocated in a dedicated memory context
|
||||
* child of TopMemoryContext, deleted at the end of pg_backup_stop(). If
|
||||
* an error happens before ending the backup, memory would be leaked in
|
||||
* this context until pg_backup_start() is called again.
|
||||
*/
|
||||
oldcontext = MemoryContextSwitchTo(TopMemoryContext);
|
||||
|
||||
/* Allocate backup state or reset it, if it comes from a previous run */
|
||||
if (backup_state == NULL)
|
||||
backup_state = (BackupState *) palloc0(sizeof(BackupState));
|
||||
else
|
||||
MemSet(backup_state, 0, sizeof(BackupState));
|
||||
|
||||
/*
|
||||
* tablespace_map may have been created in a previous backup, so take this
|
||||
* occasion to clean it.
|
||||
*/
|
||||
if (tablespace_map != NULL)
|
||||
if (backupcontext == NULL)
|
||||
{
|
||||
pfree(tablespace_map->data);
|
||||
pfree(tablespace_map);
|
||||
backupcontext = AllocSetContextCreate(TopMemoryContext,
|
||||
"on-line backup context",
|
||||
ALLOCSET_START_SMALL_SIZES);
|
||||
}
|
||||
else
|
||||
{
|
||||
backup_state = NULL;
|
||||
tablespace_map = NULL;
|
||||
MemoryContextReset(backupcontext);
|
||||
}
|
||||
|
||||
oldcontext = MemoryContextSwitchTo(backupcontext);
|
||||
backup_state = (BackupState *) palloc0(sizeof(BackupState));
|
||||
tablespace_map = makeStringInfo();
|
||||
MemoryContextSwitchTo(oldcontext);
|
||||
|
||||
@ -157,13 +159,14 @@ pg_backup_stop(PG_FUNCTION_ARGS)
|
||||
values[2] = CStringGetTextDatum(tablespace_map->data);
|
||||
|
||||
/* Deallocate backup-related variables */
|
||||
pfree(backup_state);
|
||||
backup_state = NULL;
|
||||
pfree(tablespace_map->data);
|
||||
pfree(tablespace_map);
|
||||
tablespace_map = NULL;
|
||||
pfree(backup_label);
|
||||
|
||||
/* Clean up the session-level state and its memory context */
|
||||
backup_state = NULL;
|
||||
tablespace_map = NULL;
|
||||
MemoryContextDelete(backupcontext);
|
||||
backupcontext = NULL;
|
||||
|
||||
/* Returns the record as Datum */
|
||||
PG_RETURN_DATUM(HeapTupleGetDatum(heap_form_tuple(tupdesc, values, nulls)));
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user