public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: Andres Freund <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: USE_BARRIER_SMGRRELEASE on Linux?
Date: Wed, 16 Feb 2022 09:37:21 -0800
Message-ID: <20220216173721.GA3007497@nathanxps13> (raw)
In-Reply-To: <20220216164442.GA2935315@nathanxps13>
References: <20220215235845.GA2665318@nathanxps13>
	<CA+hUKGJxb5ug=sBL=S8WMsFg4mojccy8p1VWqXYEz4MSre_vGA@mail.gmail.com>
	<CA+hUKG+Ye6kBWkm9kjMhvAvO0gyrEy=q3rm05XSmZ2ah6auQNw@mail.gmail.com>
	<20220216045911.GA2791824@nathanxps13>
	<[email protected]>
	<20220216061404.GA2813338@nathanxps13>
	<20220216065732.GA2932808@nathanxps13>
	<20220216164442.GA2935315@nathanxps13>

On Wed, Feb 16, 2022 at 08:44:42AM -0800, Nathan Bossart wrote:
> On Tue, Feb 15, 2022 at 10:57:32PM -0800, Nathan Bossart wrote:
>> On Tue, Feb 15, 2022 at 10:14:04PM -0800, Nathan Bossart wrote:
>>> It looks like register_unlink_segment() is called prior to the checkpoint,
>>> but the checkpointer is not calling RememberSyncRequest() until after
>>> SyncPreCheckpoint().  This means that the requests are registered with the
>>> next checkpoint cycle count, so they aren't processed until the next
>>> checkpoint.
>> 
>> Calling AbsorbSyncRequests() before advancing the checkpoint cycle counter
>> seems to fix the issue.  However, this requires moving SyncPreCheckpoint()
>> out of the critical section in CreateCheckPoint().  Patch attached.
> 
> An alternative fix might be to call AbsorbSyncRequests() after increasing
> the ckpt_started counter in CheckpointerMain().  AFAICT there is a window
> just before checkpointing where new requests are registered for the
> checkpoint following the one about to begin.

Here's a patch that adds a call to AbsorbSyncRequests() in
CheckpointerMain() instead of SyncPreCheckpoint().  I've also figured out a
way to reproduce the issue without the pre-allocation patches applied:

1. In checkpointer.c, add a 30 second sleep before acquiring ckpt_lck to
   increment ckpt_started.
2. In session 1, run the following commands:
	a. CREATE TABLESPACE test LOCATION '/path/to/dir';
	b. CREATE TABLE test TABLESPACE test AS SELECT 1;
3. In session 2, start a checkpoint.
4. In session 1, run these commands:
	a. ALTER TABLE test SET TABLESPACE pg_default;
	b. DROP TABLESPACE test;  -- fails
	c. DROP TABLESPACE test;  -- succeeds

With the attached patch applied, the first attempt at dropping the
tablespace no longer fails.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com


Attachments:

  [text/x-diff] v2-0001-call-AbsorbSyncRequests-after-indicating-checkpoi.patch (1.4K, ../20220216173721.GA3007497@nathanxps13/2-v2-0001-call-AbsorbSyncRequests-after-indicating-checkpoi.patch)
  download | inline diff:
From e9707dfde25eaa9c447032c4b5a61e3011141dc9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 16 Feb 2022 09:26:08 -0800
Subject: [PATCH v2 1/1] call AbsorbSyncRequests() after indicating checkpoint
 start

---
 src/backend/postmaster/checkpointer.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 4488e3a443..e93d34b71f 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -401,6 +401,18 @@ CheckpointerMain(void)
 
 			ConditionVariableBroadcast(&CheckpointerShmem->start_cv);
 
+			/*
+			 * Pick up any last minute requests.  DROP TABLESPACE schedules a
+			 * checkpoint to clean up any lingering files that are scheduled for
+			 * deletion.  If we don't absorb those requests now, they might not
+			 * be absorbed until after incrementing the checkpoint cycle
+			 * counter, so the files won't be deleted until the following
+			 * checkpoint.  By absorbing requests after indicating the
+			 * checkpoint has started, operations like DROP TABLESPACE can be
+			 * sure that the next checkpoint will clean up any such files.
+			 */
+			AbsorbSyncRequests();
+
 			/*
 			 * The end-of-recovery checkpoint is a real checkpoint that's
 			 * performed while we're still in recovery.
-- 
2.25.1



view thread (15+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: USE_BARRIER_SMGRRELEASE on Linux?
  In-Reply-To: <20220216173721.GA3007497@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox