On Tue, Jul 26, 2022 at 5:50 PM David Steele <da...@pgmasters.net> wrote:
>
> >> I would prefer to have all the components of backup_label stored
> >> separately and then generate backup_label from them in pg_backup_stop().
> >
> > +1, because pg_backup_stop is the one that's returning backup_label
> > contents, so it does make sense for it to prepare it once and for all
> > and return.
> >
> >> For PG16 I am planning to add some fields to backup_label that are not
> >> known when pg_backup_start() is called, e.g. min recovery time.
> >
> > Can you please point to your patch that does above?
>
> Currently it is a plan, not a patch. So there is nothing to show yet.
>
> > Yes, right now, backup_label or tablespace_map contents are being
> > filled in by pg_backup_start and are never changed again. But if your
> > above proposal is for fixing some issue, then it would make sense for
> > us to carry all the info in a structure to pg_backup_stop and then let
> > it prepare the backup_label and tablespace_map contents.
>
> I think this makes sense even if I don't get these changes into PG16.
>
> > If the approach is okay for the hackers, I would like to spend time on it.
>
> +1 from me.
Here comes the v1 patch. This patch tries to refactor backup related
code, advantages of doing so are following:
1) backup state is more structured now - all in a single structure,
callers can create backup_label contents whenever required, either
during the pg_backup_start or the pg_backup_stop or in between.
2) no parsing of backup_label file lines now in pg_backup_stop, no
error checking for invalid parsing.
3) backup_label and history file contents have most of the things in
common, they can now be created within a single function.
4) makes backup related code extensible and readable.
One downside is that it creates a lot of diff with previous versions.
Please review.
--
Bharath Rupireddy
RDS Open Source Databases: https://aws.amazon.com/
From 441f4c967187e52cc5e2b98046886c91105213cb Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Fri, 29 Jul 2022 23:57:12 +0000
Subject: [PATCH v1] Refactor backup related code
This patch tries to refactor backup related code, advantages of
doing so are following:
1) backup state is more structured now - all in a single structure,
callers can create backup_label contents whenever required, either
during the pg_backup_start or the pg_backup_stop or in between.
2) no parsing of backup_label file lines now, no error checking
for invalid parsing.
3) backup_label and history file contents have most of the things
in common, they can now be created within a single function
4) makes backup related code extensible and readable
---
src/backend/access/transam/xlog.c | 318 +++++++++++++------------
src/backend/access/transam/xlogfuncs.c | 48 ++--
src/backend/replication/basebackup.c | 31 ++-
src/include/access/xlog.h | 12 +-
src/include/access/xlog_internal.h | 25 ++
5 files changed, 234 insertions(+), 200 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 15ab8d90d4..4024eb6320 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8061,62 +8061,137 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
PendingWalStats.wal_sync++;
}
+/*
+ * Get backup state.
+ */
+BackupState
+get_backup_state(const char *name)
+{
+ BackupState state;
+ Size len;
+
+ state = (BackupState) palloc0(sizeof(BackupStateData));
+ len = strlen(name);
+ state->name = (char *) palloc0(len + 1);
+ memcpy(state->name, name, len);
+ state->backup_label = makeStringInfo();
+ state->tablespace_map = makeStringInfo();
+ state->history_file = makeStringInfo();
+
+ return state;
+}
+
+/*
+ * Free backup state.
+ */
+void
+free_backup_state(BackupState state)
+{
+ Assert(state != NULL);
+
+ pfree(state->name);
+ pfree(state->backup_label->data);
+ pfree(state->tablespace_map->data);
+ pfree(state->history_file->data);
+ pfree(state);
+}
+
+/*
+ * Construct backup_label or history file content strings.
+ */
+void
+create_backup_content_str(BackupState state, bool forhistoryfile)
+{
+ StringInfo str;
+ char startstrbuf[128];
+ char stopstrfbuf[128];
+
+ if (forhistoryfile)
+ str = state->history_file;
+ else
+ str = state->backup_label;
+
+ /* Use the log timezone here, not the session timezone */
+ pg_strftime(startstrbuf, sizeof(startstrbuf), "%Y-%m-%d %H:%M:%S %Z",
+ pg_localtime(&state->starttime, log_timezone));
+
+ appendStringInfo(str, "START WAL LOCATION: %X/%X (file %s)\n",
+ LSN_FORMAT_ARGS(state->startpoint),
+ state->startxlogfile);
+
+ if (forhistoryfile)
+ appendStringInfo(str, "STOP WAL LOCATION: %X/%X (file %s)\n",
+ LSN_FORMAT_ARGS(state->startpoint),
+ state->stopxlogfile);
+
+ appendStringInfo(str, "CHECKPOINT LOCATION: %X/%X\n",
+ LSN_FORMAT_ARGS(state->checkpointloc));
+ appendStringInfo(str, "BACKUP METHOD: streamed\n");
+ appendStringInfo(str, "BACKUP FROM: %s\n",
+ state->started_in_recovery ? "standby" : "primary");
+ appendStringInfo(str, "START TIME: %s\n", startstrbuf);
+ appendStringInfo(str, "LABEL: %s\n", state->name);
+ appendStringInfo(str, "START TIMELINE: %u\n", state->starttli);
+
+ if (forhistoryfile)
+ {
+ /* Use the log timezone here, not the session timezone */
+ pg_strftime(stopstrfbuf, sizeof(stopstrfbuf), "%Y-%m-%d %H:%M:%S %Z",
+ pg_localtime(&state->stoptime, log_timezone));
+
+ appendStringInfo(str, "STOP TIME: %s\n", stopstrfbuf);
+ appendStringInfo(str, "STOP TIMELINE: %u\n", state->stoptli);
+ }
+}
+
/*
* do_pg_backup_start is the workhorse of the user-visible pg_backup_start()
* function. It creates the necessary starting checkpoint and constructs the
- * backup label and tablespace map.
- *
- * Input parameters are "backupidstr" (the backup label string) and "fast"
- * (if true, we do the checkpoint in immediate mode to make it faster).
+ * backup state.
*
- * The backup label and tablespace map contents are appended to *labelfile and
- * *tblspcmapfile, and the caller is responsible for including them in the
- * backup archive as 'backup_label' and 'tablespace_map'.
- * tblspcmapfile is required mainly for tar format in windows as native windows
- * utilities are not able to create symlinks while extracting files from tar.
- * However for consistency and platform-independence, we do it the same way
- * everywhere.
+ * Input parameters are "state" (containing backup state), "fast" (if true,
+ * we do the checkpoint in immediate mode to make it faster) and "tablespaces"
+ * (if non-NULL, indicates a list of tablespaceinfo structs describing the
+ * cluster's tablespaces.)
*
- * If "tablespaces" isn't NULL, it receives a list of tablespaceinfo structs
- * describing the cluster's tablespaces.
+ * The tablespace map contents are appended to backup state's tablespace_map
+ * and the caller is responsible for including it in the backup archive as
+ * 'tablespace_map'. The tablespace_map file is required mainly for tar format
+ * in windows as native windows utilities are not able to create symlinks while
+ * extracting files from tar. However for consistency and
+ * platform-independence, we do it the same way everywhere.
*
* Returns the minimum WAL location that must be present to restore from this
- * backup, and the corresponding timeline ID in *starttli_p.
+ * backup, and the corresponding timeline ID via backup state startpoint and
+ * starttli respectively.
*
* Every successfully started backup must be stopped by calling
- * do_pg_backup_stop() or do_pg_abort_backup(). There can be many
- * backups active at the same time.
+ * do_pg_backup_stop() or do_pg_abort_backup(). There can be many backups
+ * active at the same time.
*
* It is the responsibility of the caller of this function to verify the
* permissions of the calling user!
*/
-XLogRecPtr
-do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
- StringInfo labelfile, List **tablespaces,
- StringInfo tblspcmapfile)
+void
+do_pg_backup_start(BackupState state, bool fast, List **tablespaces)
{
- bool backup_started_in_recovery = false;
- XLogRecPtr checkpointloc;
- XLogRecPtr startpoint;
- TimeLineID starttli;
- pg_time_t stamp_time;
- char strfbuf[128];
- char xlogfilename[MAXFNAMELEN];
XLogSegNo _logSegNo;
- backup_started_in_recovery = RecoveryInProgress();
+ Assert(state != NULL);
+
+ state->started_in_recovery = RecoveryInProgress();
/*
* During recovery, we don't need to check WAL level. Because, if WAL
* level is not sufficient, it's impossible to get here during recovery.
*/
- if (!backup_started_in_recovery && !XLogIsNeeded())
+ if (!state->started_in_recovery && !XLogIsNeeded())
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAL level not sufficient for making an online backup"),
errhint("wal_level must be set to \"replica\" or \"logical\" at server start.")));
- if (strlen(backupidstr) > MAXPGPATH)
+ if (strlen(state->name) > MAXPGPATH)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("backup label too long (max %d bytes)",
@@ -8178,7 +8253,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
* the backup taken during recovery is not available for the special
* recovery case described above.
*/
- if (!backup_started_in_recovery)
+ if (!state->started_in_recovery)
RequestXLogSwitch(false);
do
@@ -8213,13 +8288,13 @@ do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
* pointer.
*/
LWLockAcquire(ControlFileLock, LW_SHARED);
- checkpointloc = ControlFile->checkPoint;
- startpoint = ControlFile->checkPointCopy.redo;
- starttli = ControlFile->checkPointCopy.ThisTimeLineID;
+ state->checkpointloc = ControlFile->checkPoint;
+ state->startpoint = ControlFile->checkPointCopy.redo;
+ state->starttli = ControlFile->checkPointCopy.ThisTimeLineID;
checkpointfpw = ControlFile->checkPointCopy.fullPageWrites;
LWLockRelease(ControlFileLock);
- if (backup_started_in_recovery)
+ if (state->started_in_recovery)
{
XLogRecPtr recptr;
@@ -8232,7 +8307,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
recptr = XLogCtl->lastFpwDisableRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
- if (!checkpointfpw || startpoint <= recptr)
+ if (!checkpointfpw || state->startpoint <= recptr)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAL generated with full_page_writes=off was replayed "
@@ -8264,16 +8339,17 @@ do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
* either because only few buffers have been dirtied yet.
*/
WALInsertLockAcquireExclusive();
- if (XLogCtl->Insert.lastBackupStart < startpoint)
+ if (XLogCtl->Insert.lastBackupStart < state->startpoint)
{
- XLogCtl->Insert.lastBackupStart = startpoint;
+ XLogCtl->Insert.lastBackupStart = state->startpoint;
gotUniqueStartpoint = true;
}
WALInsertLockRelease();
} while (!gotUniqueStartpoint);
- XLByteToSeg(startpoint, _logSegNo, wal_segment_size);
- XLogFileName(xlogfilename, starttli, _logSegNo, wal_segment_size);
+ XLByteToSeg(state->startpoint, _logSegNo, wal_segment_size);
+ XLogFileName(state->startxlogfile, state->starttli, _logSegNo,
+ wal_segment_size);
/*
* Construct tablespace_map file.
@@ -8354,7 +8430,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
if (tablespaces)
*tablespaces = lappend(*tablespaces, ti);
- appendStringInfo(tblspcmapfile, "%s %s\n",
+ appendStringInfo(state->tablespace_map, "%s %s\n",
ti->oid, escapedpath.data);
pfree(escapedpath.data);
@@ -8372,25 +8448,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
}
FreeDir(tblspcdir);
- /*
- * Construct backup label file.
- */
-
- /* Use the log timezone here, not the session timezone */
- stamp_time = (pg_time_t) time(NULL);
- pg_strftime(strfbuf, sizeof(strfbuf),
- "%Y-%m-%d %H:%M:%S %Z",
- pg_localtime(&stamp_time, log_timezone));
- appendStringInfo(labelfile, "START WAL LOCATION: %X/%X (file %s)\n",
- LSN_FORMAT_ARGS(startpoint), xlogfilename);
- appendStringInfo(labelfile, "CHECKPOINT LOCATION: %X/%X\n",
- LSN_FORMAT_ARGS(checkpointloc));
- appendStringInfo(labelfile, "BACKUP METHOD: streamed\n");
- appendStringInfo(labelfile, "BACKUP FROM: %s\n",
- backup_started_in_recovery ? "standby" : "primary");
- appendStringInfo(labelfile, "START TIME: %s\n", strfbuf);
- appendStringInfo(labelfile, "LABEL: %s\n", backupidstr);
- appendStringInfo(labelfile, "START TIMELINE: %u\n", starttli);
+ state->starttime = (pg_time_t) time(NULL);
}
PG_END_ENSURE_ERROR_CLEANUP(pg_backup_start_callback, (Datum) 0);
@@ -8398,13 +8456,6 @@ do_pg_backup_start(const char *backupidstr, bool fast, TimeLineID *starttli_p,
* Mark that the start phase has correctly finished for the backup.
*/
sessionBackupState = SESSION_BACKUP_RUNNING;
-
- /*
- * We're done. As a convenience, return the starting WAL location.
- */
- if (starttli_p)
- *starttli_p = starttli;
- return startpoint;
}
/* Error cleanup callback for pg_backup_start */
@@ -8436,48 +8487,39 @@ get_backup_status(void)
/*
* do_pg_backup_stop
*
- * Utility function called at the end of an online backup. It cleans up the
- * backup state and can optionally wait for WAL segments to be archived.
+ * Utility function called at the end of an online backup. It creates backup
+ * label contents and history file (if required), resets sessionBackupState
+ * and so on. It can optionally wait for WAL segments to be archived.
*
* Returns the last WAL location that must be present to restore from this
- * backup, and the corresponding timeline ID in *stoptli_p.
+ * backup, and the corresponding timeline ID via backup state stoppoint and
+ * stoptli respectively.
*
* It is the responsibility of the caller of this function to verify the
* permissions of the calling user!
*/
-XLogRecPtr
-do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
+void
+do_pg_backup_stop(BackupState state, bool waitforarchive)
{
- bool backup_started_in_recovery = false;
- XLogRecPtr startpoint;
- XLogRecPtr stoppoint;
- TimeLineID stoptli;
- pg_time_t stamp_time;
- char strfbuf[128];
+ bool in_recovery = false;
char histfilepath[MAXPGPATH];
- char startxlogfilename[MAXFNAMELEN];
- char stopxlogfilename[MAXFNAMELEN];
char lastxlogfilename[MAXFNAMELEN];
char histfilename[MAXFNAMELEN];
- char backupfrom[20];
XLogSegNo _logSegNo;
FILE *fp;
- char ch;
int seconds_before_warning;
int waits = 0;
bool reported_waiting = false;
- char *remaining;
- char *ptr;
- uint32 hi,
- lo;
- backup_started_in_recovery = RecoveryInProgress();
+ Assert(state != NULL);
+
+ in_recovery = RecoveryInProgress();
/*
* During recovery, we don't need to check WAL level. Because, if WAL
* level is not sufficient, it's impossible to get here during recovery.
*/
- if (!backup_started_in_recovery && !XLogIsNeeded())
+ if (!in_recovery && !XLogIsNeeded())
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAL level not sufficient for making an online backup"),
@@ -8518,29 +8560,10 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
WALInsertLockRelease();
/*
- * Read and parse the START WAL LOCATION line (this code is pretty crude,
- * but we are not expecting any variability in the file format).
+ * If we are taking an online backup from the standby, we confirm that the
+ * standby has not been promoted during the backup.
*/
- if (sscanf(labelfile, "START WAL LOCATION: %X/%X (file %24s)%c",
- &hi, &lo, startxlogfilename,
- &ch) != 4 || ch != '\n')
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
- startpoint = ((uint64) hi) << 32 | lo;
- remaining = strchr(labelfile, '\n') + 1; /* %n is not portable enough */
-
- /*
- * Parse the BACKUP FROM line. If we are taking an online backup from the
- * standby, we confirm that the standby has not been promoted during the
- * backup.
- */
- ptr = strstr(remaining, "BACKUP FROM:");
- if (!ptr || sscanf(ptr, "BACKUP FROM: %19s\n", backupfrom) != 1)
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
- if (strcmp(backupfrom, "standby") == 0 && !backup_started_in_recovery)
+ if (state->started_in_recovery == true && in_recovery == false)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("the standby was promoted during online backup"),
@@ -8576,7 +8599,7 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
* an archiver is not invoked. So it doesn't seem worthwhile to write a
* backup history file during recovery.
*/
- if (backup_started_in_recovery)
+ if (in_recovery)
{
XLogRecPtr recptr;
@@ -8588,7 +8611,7 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
recptr = XLogCtl->lastFpwDisableRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
- if (startpoint <= recptr)
+ if (state->startpoint <= recptr)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAL generated with full_page_writes=off was replayed "
@@ -8600,8 +8623,8 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
LWLockAcquire(ControlFileLock, LW_SHARED);
- stoppoint = ControlFile->minRecoveryPoint;
- stoptli = ControlFile->minRecoveryPointTLI;
+ state->stoppoint = ControlFile->minRecoveryPoint;
+ state->stoptli = ControlFile->minRecoveryPointTLI;
LWLockRelease(ControlFileLock);
}
else
@@ -8610,14 +8633,15 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
* Write the backup-end xlog record
*/
XLogBeginInsert();
- XLogRegisterData((char *) (&startpoint), sizeof(startpoint));
- stoppoint = XLogInsert(RM_XLOG_ID, XLOG_BACKUP_END);
+ XLogRegisterData((char *) (&state->startpoint),
+ sizeof(state->startpoint));
+ state->stoppoint = XLogInsert(RM_XLOG_ID, XLOG_BACKUP_END);
/*
* Given that we're not in recovery, InsertTimeLineID is set and can't
* change, so we can read it without a lock.
*/
- stoptli = XLogCtl->InsertTimeLineID;
+ state->stoptli = XLogCtl->InsertTimeLineID;
/*
* Force a switch to a new xlog segment file, so that the backup is
@@ -8625,39 +8649,30 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
*/
RequestXLogSwitch(false);
- XLByteToPrevSeg(stoppoint, _logSegNo, wal_segment_size);
- XLogFileName(stopxlogfilename, stoptli, _logSegNo, wal_segment_size);
+ XLByteToPrevSeg(state->stoppoint, _logSegNo, wal_segment_size);
+ XLogFileName(state->stopxlogfile, state->stoptli, _logSegNo,
+ wal_segment_size);
- /* Use the log timezone here, not the session timezone */
- stamp_time = (pg_time_t) time(NULL);
- pg_strftime(strfbuf, sizeof(strfbuf),
- "%Y-%m-%d %H:%M:%S %Z",
- pg_localtime(&stamp_time, log_timezone));
+ state->stoptime = (pg_time_t) time(NULL);
/*
- * Write the backup history file
+ * Write the backup history file. Add backup_label file contents too it.
*/
- XLByteToSeg(startpoint, _logSegNo, wal_segment_size);
- BackupHistoryFilePath(histfilepath, stoptli, _logSegNo,
- startpoint, wal_segment_size);
+ XLByteToSeg(state->startpoint, _logSegNo, wal_segment_size);
+ BackupHistoryFilePath(histfilepath, state->stoptli, _logSegNo,
+ state->startpoint, wal_segment_size);
fp = AllocateFile(histfilepath, "w");
if (!fp)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create file \"%s\": %m",
histfilepath)));
- fprintf(fp, "START WAL LOCATION: %X/%X (file %s)\n",
- LSN_FORMAT_ARGS(startpoint), startxlogfilename);
- fprintf(fp, "STOP WAL LOCATION: %X/%X (file %s)\n",
- LSN_FORMAT_ARGS(stoppoint), stopxlogfilename);
- /*
- * Transfer remaining lines including label and start timeline to
- * history file.
- */
- fprintf(fp, "%s", remaining);
- fprintf(fp, "STOP TIME: %s\n", strfbuf);
- fprintf(fp, "STOP TIMELINE: %u\n", stoptli);
+ /* construct history file contents */
+ create_backup_content_str(state, true);
+
+ fprintf(fp, "%s", state->history_file->data);
+
if (fflush(fp) || ferror(fp) || FreeFile(fp))
ereport(ERROR,
(errcode_for_file_access(),
@@ -8673,6 +8688,9 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
CleanupBackupHistory();
}
+ /* construct backup_label contents */
+ create_backup_content_str(state, false);
+
/*
* If archiving is enabled, wait for all the required WAL files to be
* archived before returning. If archiving isn't enabled, the required WAL
@@ -8695,15 +8713,16 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
*/
if (waitforarchive &&
- ((!backup_started_in_recovery && XLogArchivingActive()) ||
- (backup_started_in_recovery && XLogArchivingAlways())))
+ ((!in_recovery && XLogArchivingActive()) ||
+ (in_recovery && XLogArchivingAlways())))
{
- XLByteToPrevSeg(stoppoint, _logSegNo, wal_segment_size);
- XLogFileName(lastxlogfilename, stoptli, _logSegNo, wal_segment_size);
+ XLByteToPrevSeg(state->stoppoint, _logSegNo, wal_segment_size);
+ XLogFileName(lastxlogfilename, state->stoptli, _logSegNo,
+ wal_segment_size);
- XLByteToSeg(startpoint, _logSegNo, wal_segment_size);
- BackupHistoryFileName(histfilename, stoptli, _logSegNo,
- startpoint, wal_segment_size);
+ XLByteToSeg(state->startpoint, _logSegNo, wal_segment_size);
+ BackupHistoryFileName(histfilename, state->stoptli, _logSegNo,
+ state->startpoint, wal_segment_size);
seconds_before_warning = 60;
waits = 0;
@@ -8744,13 +8763,6 @@ do_pg_backup_stop(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
else if (waitforarchive)
ereport(NOTICE,
(errmsg("WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup")));
-
- /*
- * We're done. As a convenience, return the ending WAL location.
- */
- if (stoptli_p)
- *stoptli_p = stoptli;
- return stoppoint;
}
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 61e0f4a29c..1c6b9e8b06 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -38,11 +38,7 @@
#include "utils/timestamp.h"
#include "utils/tuplestore.h"
-/*
- * Store label file and tablespace map during backups.
- */
-static StringInfo label_file;
-static StringInfo tblspc_map_file;
+static BackupState backup_state = NULL;
/*
* pg_backup_start: set up for taking an on-line backup dump
@@ -62,7 +58,6 @@ pg_backup_start(PG_FUNCTION_ARGS)
text *backupid = PG_GETARG_TEXT_PP(0);
bool fast = PG_GETARG_BOOL(1);
char *backupidstr;
- XLogRecPtr startpoint;
SessionBackupState status = get_backup_status();
MemoryContext oldcontext;
@@ -74,20 +69,18 @@ pg_backup_start(PG_FUNCTION_ARGS)
errmsg("a backup is already in progress in this session")));
/*
- * Label file and tablespace map file need to be long-lived, since they
- * are read in pg_backup_stop.
+ * backup_state need to be long-lived as its contents are read in
+ * pg_backup_stop, hence allocate in TopMemoryContext.
*/
oldcontext = MemoryContextSwitchTo(TopMemoryContext);
- label_file = makeStringInfo();
- tblspc_map_file = makeStringInfo();
+ backup_state = get_backup_state(backupidstr);
MemoryContextSwitchTo(oldcontext);
register_persistent_abort_backup_handler();
- startpoint = do_pg_backup_start(backupidstr, fast, NULL, label_file,
- NULL, tblspc_map_file);
+ do_pg_backup_start(backup_state, fast, NULL);
- PG_RETURN_LSN(startpoint);
+ PG_RETURN_LSN(backup_state->startpoint);
}
@@ -104,13 +97,11 @@ pg_backup_start(PG_FUNCTION_ARGS)
Datum
pg_backup_stop(PG_FUNCTION_ARGS)
{
-#define PG_STOP_BACKUP_V2_COLS 3
+#define PG_BACKUP_STOP_V2_COLS 3
TupleDesc tupdesc;
- Datum values[PG_STOP_BACKUP_V2_COLS] = {0};
- bool nulls[PG_STOP_BACKUP_V2_COLS] = {0};
-
+ Datum values[PG_BACKUP_STOP_V2_COLS] = {0};
+ bool nulls[PG_BACKUP_STOP_V2_COLS] = {0};
bool waitforarchive = PG_GETARG_BOOL(0);
- XLogRecPtr stoppoint;
SessionBackupState status = get_backup_status();
/* Initialize attributes information in the tuple descriptor */
@@ -127,19 +118,14 @@ pg_backup_stop(PG_FUNCTION_ARGS)
* Stop the backup. Return a copy of the backup label and tablespace map
* so they can be written to disk by the caller.
*/
- stoppoint = do_pg_backup_stop(label_file->data, waitforarchive, NULL);
-
- values[0] = LSNGetDatum(stoppoint);
- values[1] = CStringGetTextDatum(label_file->data);
- values[2] = CStringGetTextDatum(tblspc_map_file->data);
-
- /* Free structures allocated in TopMemoryContext */
- pfree(label_file->data);
- pfree(label_file);
- label_file = NULL;
- pfree(tblspc_map_file->data);
- pfree(tblspc_map_file);
- tblspc_map_file = NULL;
+ do_pg_backup_stop(backup_state, waitforarchive);
+
+ values[0] = LSNGetDatum(backup_state->stoppoint);
+ values[1] = CStringGetTextDatum(backup_state->backup_label->data);
+ values[2] = CStringGetTextDatum(backup_state->tablespace_map->data);
+
+ free_backup_state(backup_state);
+ backup_state = NULL;
/* Returns the record as Datum */
PG_RETURN_DATUM(HeapTupleGetDatum(heap_form_tuple(tupdesc, values, nulls)));
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 7f85071229..31924d3257 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -230,9 +230,8 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
bbsink_state state;
XLogRecPtr endptr;
TimeLineID endtli;
- StringInfo labelfile;
- StringInfo tblspc_map_file;
backup_manifest_info manifest;
+ BackupState backup_state;
/* Initial backup state, insofar as we know it now. */
state.tablespaces = NIL;
@@ -247,18 +246,18 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
backup_started_in_recovery = RecoveryInProgress();
- labelfile = makeStringInfo();
- tblspc_map_file = makeStringInfo();
InitializeBackupManifest(&manifest, opt->manifest,
opt->manifest_checksum_type);
total_checksum_failures = 0;
+ backup_state = get_backup_state(opt->label);
basebackup_progress_wait_checkpoint();
- state.startptr = do_pg_backup_start(opt->label, opt->fastcheckpoint,
- &state.starttli,
- labelfile, &state.tablespaces,
- tblspc_map_file);
+ do_pg_backup_start(backup_state, opt->fastcheckpoint,
+ &state.tablespaces);
+
+ state.startptr = backup_state->startpoint;
+ state.starttli = backup_state->starttli;
/*
* Once do_pg_backup_start has been called, ensure that any failure causes
@@ -315,14 +314,19 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
bbsink_begin_archive(sink, "base.tar");
+ /* construct backup_label contents */
+ create_backup_content_str(backup_state, false);
+
/* In the main tar, include the backup_label first... */
- sendFileWithContent(sink, BACKUP_LABEL_FILE, labelfile->data,
+ sendFileWithContent(sink, BACKUP_LABEL_FILE,
+ backup_state->backup_label->data,
&manifest);
/* Then the tablespace_map file, if required... */
if (opt->sendtblspcmapfile)
{
- sendFileWithContent(sink, TABLESPACE_MAP, tblspc_map_file->data,
+ sendFileWithContent(sink, TABLESPACE_MAP,
+ backup_state->tablespace_map->data,
&manifest);
sendtblspclinks = false;
}
@@ -373,7 +377,12 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
}
basebackup_progress_wait_wal_archive(&state);
- endptr = do_pg_backup_stop(labelfile->data, !opt->nowait, &endtli);
+ do_pg_backup_stop(backup_state, !opt->nowait);
+
+ endptr = backup_state->stoppoint;
+ endtli = backup_state->stoptli;
+
+ free_backup_state(backup_state);
}
PG_END_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, BoolGetDatum(false));
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index cd674c3c23..202495847c 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -11,6 +11,7 @@
#ifndef XLOG_H
#define XLOG_H
+#include "access/xlog_internal.h"
#include "access/xlogdefs.h"
#include "access/xlogreader.h"
#include "datatype/timestamp.h"
@@ -279,11 +280,12 @@ typedef enum SessionBackupState
SESSION_BACKUP_RUNNING,
} SessionBackupState;
-extern XLogRecPtr do_pg_backup_start(const char *backupidstr, bool fast,
- TimeLineID *starttli_p, StringInfo labelfile,
- List **tablespaces, StringInfo tblspcmapfile);
-extern XLogRecPtr do_pg_backup_stop(char *labelfile, bool waitforarchive,
- TimeLineID *stoptli_p);
+extern BackupState get_backup_state(const char *name);
+extern void free_backup_state(BackupState state);
+extern void create_backup_content_str(BackupState state, bool forhistoryfile);
+extern void do_pg_backup_start(BackupState state, bool fast,
+ List **tablespaces);
+extern void do_pg_backup_stop(BackupState state, bool waitforarchive);
extern void do_pg_abort_backup(int code, Datum arg);
extern void register_persistent_abort_backup_handler(void);
extern SessionBackupState get_backup_status(void);
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 44291b337b..462520342a 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -380,6 +380,31 @@ GetRmgr(RmgrId rmid)
}
#endif
+/* Structure to hold backup state. */
+typedef struct BackupStateData
+{
+ /* Following are the fields captured when the backup starts. */
+ char *name;
+ XLogRecPtr startpoint; /* backup start WAL location */
+ char startxlogfile[MAXFNAMELEN]; /* backup start WAL file */
+ TimeLineID starttli; /* backup start TLI */
+ XLogRecPtr checkpointloc; /* last checkpoint location */
+ pg_time_t starttime; /* backup start time */
+ bool started_in_recovery; /* backup started in recovery? */
+
+ StringInfo backup_label; /* backup_label file contents. */
+ StringInfo tablespace_map; /* tablespace_map file contents. */
+ StringInfo history_file; /* history file contents. */
+
+ /* Following are the fields captured during or after the backup stops. */
+ XLogRecPtr stoppoint; /* backup stop WAL location */
+ char stopxlogfile[MAXFNAMELEN]; /* backup stop WAL file */
+ TimeLineID stoptli; /* backup stop TLI */
+ pg_time_t stoptime; /* backup stop time */
+} BackupStateData;
+
+typedef BackupStateData *BackupState;
+
/*
* Exported to support xlog switching from checkpointer
*/
--
2.34.1
