Message-ID: <d77162d8-112f-c6d3-e43b-296d32b4611b@pgmasters.net>
Date: Tue, 26 Jul 2022 07:21:29 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: Race between KeepFileRestoredFromArchive() and restartpoint
Content-Language: en-US
To: Noah Misch <noah@leadboat.com>, pgsql-hackers@postgresql.org,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
References: <20210202151416.GB3304930@rfd.leadboat.com>
 <20210619203918.GA1189385@rfd.leadboat.com>
From: David Steele <david@pgmasters.net>
In-Reply-To: <20210619203918.GA1189385@rfd.leadboat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/d77162d8-112f-c6d3-e43b-296d32b4611b%40pgmasters.net>
Precedence: bulk

Hi Noah,

On 6/19/21 16:39, Noah Misch wrote:
> On Tue, Feb 02, 2021 at 07:14:16AM -0800, Noah Misch wrote:
>> Recycling and preallocation are wasteful during archive recovery, because
>> KeepFileRestoredFromArchive() unlinks every entry in its path.  I propose to
>> fix the race by adding an XLogCtl flag indicating which regime currently owns
>> the right to add long-term pg_wal directory entries.  In the archive recovery
>> regime, the checkpointer will not preallocate and will unlink old segments
>> instead of recycling them (like wal_recycle=off).  XLogFileInit() will fail.
> 
> Here's the implementation.  Patches 1-4 suffice to stop the user-visible
> ERROR.  Patch 5 avoids a spurious LOG-level message and wasted filesystem
> writes, and it provides some future-proofing.
> 
> I was tempted to (but did not) just remove preallocation.  Creating one file
> per checkpoint seems tiny relative to the max_wal_size=1GB default, so I
> expect it's hard to isolate any benefit.  Under the old checkpoint_segments=3
> default, a preallocated segment covered a respectable third of the next
> checkpoint.  Before commit 63653f7 (2002), preallocation created more files.

This also seems like it would fix the link issues we are seeing in [1].

I wonder if that would make it worth a back patch?

[1] 
https://www.postgresql.org/message-id/flat/CAKw-smBhLOGtRJTC5c%3DqKTPz8gz5%2BWPoVAXrHB6mY-1U4_N7-w%40mail.gmail.com