Skip to content

Commit 8de6278

Browse files
feodorfeodor
committed
Fix backup canceling
Assert-enabled build crashes but without asserts it works by wrong way: it may not reset forcing full page write and preventing from starting exclusive backup with the same name as cancelled. Patch replaces pair of booleans nonexclusive_backup_running/exclusive_backup_running to single enum to correctly describe backup state. Backpatch to 9.6 where bug was introduced Reported-by: David Steele Authors: Michael Paquier, David Steele Reviewed-by: Anastasia Lubennikova https://commitfest.postgresql.org/13/1068/
1 parent 4ae0805 commit 8de6278

File tree

3 files changed

+52
-16
lines changed

3 files changed

+52
-16
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,12 @@ typedef enum ExclusiveBackupState
482482
EXCLUSIVE_BACKUP_STOPPING
483483
} ExclusiveBackupState;
484484

485+
/*
486+
* Session status of running backup, used for sanity checks in SQL-callable
487+
* functions to start and stop backups.
488+
*/
489+
static SessionBackupState sessionBackupState = SESSION_BACKUP_NONE;
490+
485491
/*
486492
* Shared state data for WAL insertion.
487493
*/
@@ -10249,13 +10255,17 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1024910255

1025010256
/*
1025110257
* Mark that start phase has correctly finished for an exclusive backup.
10258+
* Session-level locks are updated as well to reflect that state.
1025210259
*/
1025310260
if (exclusive)
1025410261
{
1025510262
WALInsertLockAcquireExclusive();
1025610263
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
1025710264
WALInsertLockRelease();
10265+
sessionBackupState = SESSION_BACKUP_EXCLUSIVE;
1025810266
}
10267+
else
10268+
sessionBackupState = SESSION_BACKUP_NON_EXCLUSIVE;
1025910269

1026010270
/*
1026110271
* We're done. As a convenience, return the starting WAL location.
@@ -10310,6 +10320,15 @@ pg_stop_backup_callback(int code, Datum arg)
1031010320
WALInsertLockRelease();
1031110321
}
1031210322

10323+
/*
10324+
* Utility routine to fetch the session-level status of a backup running.
10325+
*/
10326+
SessionBackupState
10327+
get_backup_status(void)
10328+
{
10329+
return sessionBackupState;
10330+
}
10331+
1031310332
/*
1031410333
* do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
1031510334
* function.
@@ -10477,6 +10496,9 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
1047710496
}
1047810497
WALInsertLockRelease();
1047910498

10499+
/* Clean up session-level lock */
10500+
sessionBackupState = SESSION_BACKUP_NONE;
10501+
1048010502
/*
1048110503
* Read and parse the START WAL LOCATION line (this code is pretty crude,
1048210504
* but we are not expecting any variability in the file format).

‎src/backend/access/transam/xlogfuncs.c

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,6 @@
4343
*/
4444
static StringInfo label_file;
4545
static StringInfo tblspc_map_file;
46-
static bool exclusive_backup_running = false;
47-
static bool nonexclusive_backup_running = false;
4846

4947
/*
5048
* Called when the backend exits with a running non-exclusive base backup,
@@ -79,10 +77,11 @@ pg_start_backup(PG_FUNCTION_ARGS)
7977
char *backupidstr;
8078
XLogRecPtr startpoint;
8179
DIR *dir;
80+
SessionBackupState status = get_backup_status();
8281

8382
backupidstr = text_to_cstring(backupid);
8483

85-
if (exclusive_backup_running || nonexclusive_backup_running)
84+
if (status == SESSION_BACKUP_NON_EXCLUSIVE)
8685
ereport(ERROR,
8786
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
8887
errmsg("a backup is already in progress in this session")));
@@ -97,7 +96,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
9796
{
9897
startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL,
9998
dir, NULL, NULL, false, true);
100-
exclusive_backup_running = true;
10199
}
102100
else
103101
{
@@ -114,7 +112,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
114112

115113
startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file,
116114
dir, NULL, tblspc_map_file, false, true);
117-
nonexclusive_backup_running = true;
118115

119116
before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
120117
}
@@ -148,23 +145,22 @@ Datum
148145
pg_stop_backup(PG_FUNCTION_ARGS)
149146
{
150147
XLogRecPtr stoppoint;
148+
SessionBackupState status = get_backup_status();
151149

152-
if (nonexclusive_backup_running)
150+
if (status == SESSION_BACKUP_NON_EXCLUSIVE)
153151
ereport(ERROR,
154152
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
155153
errmsg("non-exclusive backup in progress"),
156154
errhint("Did you mean to use pg_stop_backup('f')?")));
157155

158156
/*
159157
* Exclusive backups were typically started in a different connection, so
160-
* don't try to verify that exclusive_backup_running is set in this one.
161-
* Actual verification that an exclusive backup is in fact running is
162-
* handled inside do_pg_stop_backup.
158+
* don't try to verify that status of backup is set to
159+
* SESSION_BACKUP_EXCLUSIVE in this function. Actual verification that an
160+
* exclusive backup is in fact running is handled inside do_pg_stop_backup.
163161
*/
164162
stoppoint = do_pg_stop_backup(NULL, true, NULL);
165163

166-
exclusive_backup_running = false;
167-
168164
PG_RETURN_LSN(stoppoint);
169165
}
170166

@@ -192,6 +188,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
192188

193189
bool exclusive = PG_GETARG_BOOL(0);
194190
XLogRecPtr stoppoint;
191+
SessionBackupState status = get_backup_status();
195192

196193
/* check to see if caller supports us returning a tuplestore */
197194
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
@@ -223,7 +220,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
223220

224221
if (exclusive)
225222
{
226-
if (nonexclusive_backup_running)
223+
if (status == SESSION_BACKUP_NON_EXCLUSIVE)
227224
ereport(ERROR,
228225
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
229226
errmsg("non-exclusive backup in progress"),
@@ -234,14 +231,13 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
234231
* return NULL for both backup_label and tablespace_map.
235232
*/
236233
stoppoint = do_pg_stop_backup(NULL, true, NULL);
237-
exclusive_backup_running = false;
238234

239235
nulls[1] = true;
240236
nulls[2] = true;
241237
}
242238
else
243239
{
244-
if (!nonexclusive_backup_running)
240+
if (status != SESSION_BACKUP_NON_EXCLUSIVE)
245241
ereport(ERROR,
246242
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
247243
errmsg("non-exclusive backup is not in progress"),
@@ -252,7 +248,6 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
252248
* and tablespace map so they can be written to disk by the caller.
253249
*/
254250
stoppoint = do_pg_stop_backup(label_file->data, true, NULL);
255-
nonexclusive_backup_running = false;
256251
cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
257252

258253
values[1] = CStringGetTextDatum(label_file->data);

‎src/include/access/xlog.h

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,15 +274,34 @@ extern void assign_max_wal_size(int newval, void *extra);
274274
extern void assign_checkpoint_completion_target(double newval, void *extra);
275275

276276
/*
277-
* Starting/stopping a base backup
277+
* Routines to start, stop, and get status of a base backup.
278278
*/
279+
280+
/*
281+
* Session-level status of base backups
282+
*
283+
* This is used in parallel with the shared memory status to control parallel
284+
* execution of base backup functions for a given session, be it a backend
285+
* dedicated to replication or a normal backend connected to a database. The
286+
* update of the session-level status happens at the same time as the shared
287+
* memory counters to keep a consistent global and local state of the backups
288+
* running.
289+
*/
290+
typedef enum SessionBackupState
291+
{
292+
SESSION_BACKUP_NONE,
293+
SESSION_BACKUP_EXCLUSIVE,
294+
SESSION_BACKUP_NON_EXCLUSIVE
295+
} SessionBackupState;
296+
279297
extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast,
280298
TimeLineID *starttli_p, StringInfo labelfile, DIR *tblspcdir,
281299
List **tablespaces, StringInfo tblspcmapfile, bool infotbssize,
282300
bool needtblspcmapfile);
283301
extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive,
284302
TimeLineID *stoptli_p);
285303
extern void do_pg_abort_backup(void);
304+
extern SessionBackupState get_backup_status(void);
286305

287306
/* File path names (all relative to $PGDATA) */
288307
#define BACKUP_LABEL_FILE "backup_label"

0 commit comments

Comments
 (0)