From 135a6ae030d4cd3f175b624c6e02d5a4084bb5f3 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 11 Dec 2019 17:28:48 -0500 Subject: [PATCH] Remove misuse of cancel_before_shmem_exit. Can cause spurious warning or assert failure. --- src/backend/access/transam/xlog.c | 14 +++++++++++--- src/backend/access/transam/xlogfuncs.c | 17 ++--------------- src/backend/replication/basebackup.c | 16 ++-------------- src/include/access/xlog.h | 2 +- 4 files changed, 16 insertions(+), 33 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 6bc1a6b46d..4cca2d3f7c 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -11136,23 +11136,27 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p) * system out of backup mode, thus making it a lot more safe to call from * an error handler. * + * The caller can pass 'arg' as 'true' or 'false' to control whether a warning + * is emitted. + * * NB: This is only for aborting a non-exclusive backup that doesn't write * backup_label. A backup started with pg_start_backup() needs to be finished * with pg_stop_backup(). */ void -do_pg_abort_backup(void) +do_pg_abort_backup(int code, Datum arg) { + bool emit_warning = DatumGetBool(arg); + /* * Quick exit if session is not keeping around a non-exclusive backup * already started. */ - if (sessionBackupState == SESSION_BACKUP_NONE) + if (sessionBackupState != SESSION_BACKUP_NON_EXCLUSIVE) return; WALInsertLockAcquireExclusive(); Assert(XLogCtl->Insert.nonExclusiveBackups > 0); - Assert(sessionBackupState == SESSION_BACKUP_NON_EXCLUSIVE); XLogCtl->Insert.nonExclusiveBackups--; if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE && @@ -11161,6 +11165,10 @@ do_pg_abort_backup(void) XLogCtl->Insert.forcePageWrites = false; } WALInsertLockRelease(); + + if (emit_warning) + ereport(WARNING, + (errmsg("aborting backup due to backend exiting before pg_stop_back up was called"))); } /* diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index 1fccf29a36..cac5854fcf 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -44,18 +44,6 @@ static StringInfo label_file; static StringInfo tblspc_map_file; -/* - * Called when the backend exits with a running non-exclusive base backup, - * to clean up state. - */ -static void -nonexclusive_base_backup_cleanup(int code, Datum arg) -{ - do_pg_abort_backup(); - ereport(WARNING, - (errmsg("aborting backup due to backend exiting before pg_stop_backup was called"))); -} - /* * pg_start_backup: set up for taking an on-line backup dump * @@ -103,10 +91,10 @@ pg_start_backup(PG_FUNCTION_ARGS) tblspc_map_file = makeStringInfo(); MemoryContextSwitchTo(oldcontext); + before_shmem_exit(do_pg_abort_backup, BoolGetDatum(true)); + startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file, NULL, tblspc_map_file, false, true); - - before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0); } PG_RETURN_LSN(startpoint); @@ -248,7 +236,6 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS) * and tablespace map so they can be written to disk by the caller. */ stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL); - cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0); values[1] = CStringGetTextDatum(label_file->data); values[2] = CStringGetTextDatum(tblspc_map_file->data); diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index 1fa4551eff..1bb72a0a57 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -65,7 +65,6 @@ static int64 _tarWriteDir(const char *pathbuf, int basepathlen, struct stat *sta bool sizeonly); static void send_int8_string(StringInfoData *buf, int64 intval); static void SendBackupHeader(List *tablespaces); -static void base_backup_cleanup(int code, Datum arg); static void perform_base_backup(basebackup_options *opt); static void parse_basebackup_options(List *options, basebackup_options *opt); static void SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli); @@ -216,17 +215,6 @@ static const char *const noChecksumFiles[] = { NULL, }; - -/* - * Called when ERROR or FATAL happens in perform_base_backup() after - * we have started the backup - make sure we end it! - */ -static void -base_backup_cleanup(int code, Datum arg) -{ - do_pg_abort_backup(); -} - /* * Actually do a base backup for the specified tablespaces. * @@ -265,7 +253,7 @@ perform_base_backup(basebackup_options *opt) * do_pg_stop_backup() should be inside the error cleanup block! */ - PG_ENSURE_ERROR_CLEANUP(base_backup_cleanup, (Datum) 0); + PG_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, BoolGetDatum(false)); { ListCell *lc; tablespaceinfo *ti; @@ -374,7 +362,7 @@ perform_base_backup(basebackup_options *opt) endptr = do_pg_stop_backup(labelfile->data, !opt->nowait, &endtli); } - PG_END_ENSURE_ERROR_CLEANUP(base_backup_cleanup, (Datum) 0); + PG_END_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, BoolGetDatum(false)); if (opt->includewal) diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 9b588c87a5..02d95f2816 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -349,7 +349,7 @@ extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast, bool needtblspcmapfile); extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p); -extern void do_pg_abort_backup(void); +extern void do_pg_abort_backup(int code, Datum arg); extern SessionBackupState get_backup_status(void); /* File path names (all relative to $PGDATA) */ -- 2.17.2 (Apple Git-113)