Re: Reset unlogged relations before syncing the data directory? - Mailing list pgsql-hackers

From Chao Li
Subject Re: Reset unlogged relations before syncing the data directory?
Date
Msg-id 766EEB3A-4F42-4331-866A-4A38277522B6@gmail.com
Whole thread
In response to Re: Reset unlogged relations before syncing the data directory?  (Ayush Tiwari <ayushtiwari.slg01@gmail.com>)
List pgsql-hackers

> On Oct 10, 2026, at 11:44, Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
>
> Hi,
>
> On Sat, 10 Oct 2026 at 07:36, Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Oct 8, 2026, at 05:20, Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> During crash startup we sync the unlogged forks, only to remove them later.
>>> ISTM we could avoid that writeback by doing the cleanup first.
>>>
>>> The attached patch puts the cleanup after InitWalRecovery() has restored
>>> any tablespace links, but before SyncDataDirectory(). Init forks and
>>> surviving files are still synced before replay, and the end-of-recovery
>>> checkpoint stays where it is.
>>>
>>> I've kept the later cleanup for recovery after a clean shutdown. We need
>>> pg_control to record recovery first, otherwise a failed start could leave
>>> missing unlogged files without forcing recovery on the next normal start.
>>>
>>> AFAICS this should help recovery_init_sync_method=syncfs too, since it
>>> can't skip individual files.
>>>
>>> I got the following startup times on a Linux VM (8 vCPUs, 32 GB RAM) with
>>> ext4, using a 1.14 GiB unlogged table (four runs each):
>>>
>>> Method  Unpatched median (range)  Patched median
>>> fsync   26.34 s (21.83-27.34)      0.40 s
>>> syncfs  22.58 s (19.43-23.93)      0.30 s
>>>
>>> The empty-table fsync control had a 0.30 s median for both builds.
>>>
>>> P.S. These runs were right after a load and pg_ctl stop -m immediate,
>>> without a manual cache flush, so there may still have been dirty data in
>>> the OS page cache at restart (which could explain such big gains)
>>> <v1-0001-Reset-unlogged-relations-before-syncing-the-data-directory.patch>
>
> Thanks for looking at this.
>
>> I agree with the optimization, though the benefit will depend on how much
>> unlogged data remains dirty in the OS cache.
>
> Agreed. These runs were right after a load and immediate shutdown; I haven't
> measured dirty pages per relation.
>
>> WRT the code change, ResetUnloggedRelations used to be called only if
>> InRecovery is true. Should we keep that explicit check, like:
>
> I've added the explicit InRecovery check in the attached v2. It's redundant
> today, as you noted, but makes it clear that cleanup is only for recovery.
> The rest of the patch is unchanged.
>
> Regards,
> Ayush
> <v2-0001-Reset-unlogged-relations-before-syncing-the-data-directory.patch>

Thanks for updating the patch. V2 LGTM.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







pgsql-hackers by date:

Previous
From: jian he
Date:
Subject: Re: [PATCH] Fix pg_dump --clean with inherited partition constraints
Next
From: Daniel Gustafsson
Date:
Subject: Re: docs: Table 9.46. UUID Extraction Functions