Date: Thu, 23 Jan 2025 12:21:12 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Andy Fan <zhihuifan1213@163.com>, Justin Pryzby <pryzby@telsasoft.com>,
	Maxim Orlov <orlovmg@gmail.com>,
	Pavel Borisov <pashkin.elfe@gmail.com>,
	"Bossart, Nathan" <bossartn@amazon.com>,
	Maxim Orlov <m.orlov@postgrespro.ru>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Pre-allocating WAL files
Message-ID: <Z5KImHaKdpqtYqb4@nathan>
References: <20220317231212.GB1415284@nathanxps13>
 <20220408203003.GA1630183@nathanxps13>
 <87sepc4zcg.fsf@163.com>
 <Z4_C04ZJ_SymvYes@nathan>
 <Z4_Hmj2R8NXnAI-K@nathan>
 <vfhyldq3rtnwd4uxbb7ro2opmvbmfswvkycavmzcqb55mmo4jk@g2d2n6xnwicb>
 <87o6zz4pld.fsf@163.com>
 <745zvagaf6tfn2zbvtmqvxsr6kmybjbzcw6rsm55jnql4233je@tudeipoewruo>
 <Z5EuOBav0sc_vG9c@nathan>
 <zwsab4baoouacwre6h63qzeiaebaaruv6etujlqxnt56knmlpv@zo654cugfyje>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <zwsab4baoouacwre6h63qzeiaebaaruv6etujlqxnt56knmlpv@zo654cugfyje>
Archived-At: <https://www.postgresql.org/message-id/Z5KImHaKdpqtYqb4%40nathan>
Precedence: bulk

On Wed, Jan 22, 2025 at 01:00:08PM -0500, Andres Freund wrote:
> On 2025-01-22 11:43:20 -0600, Nathan Bossart wrote:
>> What is the purpose of syncing the file before the rename?
> 
> It's from the general durable_rename() code. The reason it's there that it's
> required for "atomically replace a file" use case. Imagine the following:
> 
> create_and_fill("somefile.tmp");
> rename("somefile.tmp", "somefile");
> fsync("somefile.tmp");
> fsync(".");
> 
> If you crash (OS/HW level) in the wrong moment (between rename() taking effect
> in-memory and the fsyncs), you might end up with "somefile" pointing to the
> *new* file, because the rename took affect, but the new file's content not
> having reached disk yet. I.e. "somefile" will be empty.  Whether that's
> possible depends on filesystem semantics (e.g. on ext4 it's possible with
> data=writeback, I think it's always possible on xfs).
> 
> In contrast to that, if you fsync("somefile.tmp") before the rename, a crash
> between rename() and the later fsyncs will have "somefile" either pointing to
> the *old and valid contents* or the *new and valid contents*, without a chance
> for an empty file.

Got it, thanks for explaining.  If the contents are sync'd before the
rename(), do we still need to fsync() it again afterwards, too?  I'd expect
that to ordinarily not have much to do, but perhaps I'm forgetting about
some metadata that isn't covered by the fsync() on the directory.

> However, for the case of WAL recycling, we shouldn't need fsync() before the
> rename, because we ought to already have done so when creating
> (c.f. XLogFileInitInternal() or when recycling it last time.

Makes sense.

> I suspect the theoretically superfluous fsync() won't have a meaningful
> performance impact most of the time though, because
> 
> a) There shouldn't be any dirty data for the file, obviously we need to have
>    flushed the WAL past the recycled segment
> 
> b) Except for the first to-be-recycled segment, we just fsynced after the last
>    rename, so there won't be any filesystem journal data that needs to be
>    flushed
> 
> I'm not entirely sure about a) though - depending on mount options it's
> possible that the fsync() will flush file modification times when using
> wal_sync_method=fdatasync.  But even if that's possibly reachable, I doubt
> it'll be common, due to a checkpoint having to complete between the WAL flush
> and recycling. Could be worth experimenting with.

Yeah, I'm not too worried about the performance impact of some superfluous
fsync() calls, either, but I wasn't sure I properly understood the fsync()
pattern in durable_rename() (and figured it'd be nice to get it documented
in the archives).

-- 
nathan