Date: Sun, 6 Apr 2025 11:16:13 -0700
From: Noah Misch <noah@leadboat.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
	Alexander Lakhin <exclusion@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>,
	Laurenz Albe <laurenz.albe@cybertec.at>, rootcause000@gmail.com,
	pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18146: Rows reappearing in Tables after Auto-Vacuum Failure
 in PostgreSQL on Windows
Message-ID: <20250406181613.61.nmisch@google.com>
References: 
 <CA+hUKG+_ubs52OfrFBK7sH-X5pk6Y7BbwnXOgqndGKddLxZa3w@mail.gmail.com>
 <CA+hUKGL-sBkMRREO-CHM5B8iez0rxWqtr81fYM+idwNbWqTqEA@mail.gmail.com>
 <CA+hUKGLtHwNhnphLLUEUpYd2axPeRDp4D9zD5YWSdudYF-3zhQ@mail.gmail.com>
 <CA+Tgmoa1CQ5cbUoOK9dfnofCSbu6NmBaYptJ57bTe41MECQ-+Q@mail.gmail.com>
 <CA+hUKGK4omtUqvyP9rE2u=LtDUAH8WvzAp+zq9d2JG99ZajxwA@mail.gmail.com>
 <CA+hUKG+5nfWcpnZ=Z=UpGvY1tTF=4QU_0U_07EFaKmH7Nr+NLQ@mail.gmail.com>
 <9749e358-60e5-51e8-aa05-1eaf39ef339e@gmail.com>
 <8e499d50-09ac-479a-b5b2-8a8bca83d057@iki.fi>
 <00477c52-3a35-4621-b80b-58a5a48b113f@iki.fi>
 <CA+hUKG+g8ydXzSnHQPtNhmwNhn8A-FborSZGSLg62tivaugP0g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CA+hUKG+g8ydXzSnHQPtNhmwNhn8A-FborSZGSLg62tivaugP0g@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20250406181613.61.nmisch%40google.com>
Precedence: bulk

On Fri, Sep 06, 2024 at 11:05:26AM +1200, Thomas Munro wrote:
> Subject: [PATCH v3 1/3] RelationTruncate() must set DELAY_CHKPT_START.
> 
> Previously, it set only DELAY_CHKPT_COMPLETE. That was important,
> because it meant that if the XLOG_SMGR_TRUNCATE record preceded a
> XLOG_CHECKPOINT_ONLINE record in the WAL, then the truncation would also
> happen on disk before the XLOG_CHECKPOINT_ONLINE record was
> written.

I read this commit (75818b3) with intense interest, mostly to see if it found
a rule that commit 8e7e672 needs to follow and didn't (no).  I did want to
find or make an example of interleaved events where DELAY_CHKPT_COMPLETE is
still necessary.  Do any of you have such an example?  Here were my
unsuccessful attempts:

==== Attempt: from upthread

On Wed, Oct 11, 2023 at 12:59:39PM -0400, Robert Haas wrote:
> Suppose that RelationTruncate set both DELAY_CHKPT_START and
> DELAY_CHKPT_COMPLETE. I think that would prevent this problem. P2
> could still choose the redo LSN after P1 logged the truncate, but it
> wouldn't then be able to reach CheckPointBuffers() until after P1 had
> reached RegisterSyncRequest. Note that setting *only*
> DELAY_CHKPT_START isn't good enough, because then we can get this
> history:
> 
> P1: log truncate
> P2:                        choose redo LSN
> P2:                        CheckPointBuffers()
> P1: DropRelationBuffers()
> P2:                        ProcessSyncRequests()
> P2:                        log checkpoint
>               *** system loses power ***

If "choose redo LSN" happens at the point shown there, DELAY_CHKPT_START won't
allow CheckPointBuffers() at the point shown there, so this interleaving
doesn't happen.


==== Attempt: XLOG_SMGR_TRUNCATE+DropRelationBuffers() just after checkpoint waits for DELAY_CHKPT_START

P2:                        choose redo LSN 1
P2:                        CheckPointBuffers() enter
P1: DELAY_CHKPT_START enter
P1: log truncate
P1: DropRelationBuffers()
P2:                        CheckPointBuffers() actual flushes
P2:                        ProcessSyncRequests()
P2:                        log checkpoint
P2:                        choose redo LSN 2 (next checkpoint)
              *** system loses power, below were still in future ***
P1: ftruncate()
P1: RegisterSyncRequest()
P1: DELAY_CHKPT_START exit

DropRelationBuffers() made CheckPointBuffers() do less work, and the validity
of the checkpoint is now tied to truncate happening eventually.  However, if
XLOG_CHECKPOINT_ONLINE reaches disk, that implies XLOG_SMGR_TRUNCATE reached
disk first.  Any recovery starting at or before LSN 1 will redo
XLOG_SMGR_TRUNCATE.  Data integrity is fine.


==== Attempt: replay finding "older contents than expected"

A key RelationTruncate() code comment mentions this:

	 * First, the truncation operation might drop buffers that the checkpoint
	 * otherwise would have flushed. If it does, then it's essential that the
	 * files actually get truncated on disk before the checkpoint record is
	 * written. Otherwise, if replay begins from that checkpoint, the
	 * to-be-truncated blocks might still exist on disk but have older
	 * contents than expected, which can cause replay to fail. It's OK for the
	 * blocks to not exist on disk at all, but not for them to have the wrong
	 * contents. For this reason, we need to set DELAY_CHKPT_COMPLETE while
	 * this code executes.

However, I can't see how to make that happen.  RelationTruncate() has
AccessExclusiveLock on the relation, so no other WAL records for the truncated
block range are happening while we hold DELAY_CHKPT_START.  In the previous
attempt, any WAL records for the truncated block range must be before
XLOG_SMGR_TRUNCATE (such records tolerate older content) or after
XLOG_CHECKPOINT_ONLINE (since RelationTruncate() held AccessExclusiveLock at
least that late).


==== Attempt: all truncate steps just after XLOG_CHECKPOINT_REDO

P1: DELAY_CHKPT_START enter
P2:                        choose redo LSN
P1: log truncate
P1: DropRelationBuffers()
P1: ftruncate()
P1: RegisterSyncRequest()
P1: DELAY_CHKPT_START exit
P2:                        CheckPointBuffers()
P2:                        ProcessSyncRequests()
P2:                        log checkpoint
              *** system loses power ***

Recovery sees the XLOG_CHECKPOINT_ONLINE, which implies it also sees and
replays the XLOG_SMGR_TRUNCATE.  Data integrity is fine.


==== Attempt: XLOG_SMGR_TRUNCATE before XLOG_CHECKPOINT_REDO

P1: DELAY_CHKPT_START enter
P1: log truncate
P2:                        choose redo LSN
P1: DropRelationBuffers()
P1: ftruncate()
P1: RegisterSyncRequest()
P1: DELAY_CHKPT_START exit
P2:                        CheckPointBuffers()
P2:                        ProcessSyncRequests()
P2:                        log checkpoint
              *** system loses power ***

XLOG_SMGR_TRUNCATE precedes the redo point, so recovery finds no need to redo
the XLOG_SMGR_TRUNCATE.  Data integrity is fine.