From 46d9bfb0a68f7b145199711d2fb5d37561c4a130 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 16 Mar 2022 17:20:24 +1300 Subject: Fix race between DROP TABLESPACE and checkpointing. Commands like ALTER TABLE SET TABLESPACE may leave files for the next checkpoint to clean up. If such files are not removed by the time DROP TABLESPACE is called, we request a checkpoint so that they are deleted. However, there is presently a window before checkpoint start where new unlink requests won't be scheduled until the following checkpoint. This means that the checkpoint forced by DROP TABLESPACE might not remove the files we expect it to remove, and the following ERROR will be emitted: ERROR: tablespace "mytblspc" is not empty To fix, add a call to AbsorbSyncRequests() just before advancing the unlink cycle counter. This ensures that any unlink requests forwarded prior to checkpoint start (i.e., when ckpt_started is incremented) will be processed by the current checkpoint. Since AbsorbSyncRequests() performs memory allocations, it cannot be called within a critical section, so we also need to move SyncPreCheckpoint() to before CreateCheckPoint()'s critical section. This is an old bug, so back-patch to all supported versions. Author: Nathan Bossart Reported-by: Nathan Bossart Reviewed-by: Thomas Munro Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20220215235845.GA2665318%40nathanxps13 --- src/backend/access/transam/xlog.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'src/backend/access/transam/xlog.c') diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index ed16f279b1f..f436471b276 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6297,6 +6297,14 @@ CreateCheckPoint(int flags) MemSet(&CheckpointStats, 0, sizeof(CheckpointStats)); CheckpointStats.ckpt_start_t = GetCurrentTimestamp(); + /* + * Let smgr prepare for checkpoint; this has to happen outside the + * critical section and before we determine the REDO pointer. Note that + * smgr must not do anything that'd have to be undone if we decide no + * checkpoint is needed. + */ + SyncPreCheckpoint(); + /* * Use a critical section to force system panic if we have trouble. */ @@ -6310,13 +6318,6 @@ CreateCheckPoint(int flags) LWLockRelease(ControlFileLock); } - /* - * Let smgr prepare for checkpoint; this has to happen before we determine - * the REDO pointer. Note that smgr must not do anything that'd have to - * be undone if we decide no checkpoint is needed. - */ - SyncPreCheckpoint(); - /* Begin filling in the checkpoint WAL record */ MemSet(&checkPoint, 0, sizeof(checkPoint)); checkPoint.time = (pg_time_t) time(NULL); -- cgit v1.2.3