Re: Reset unlogged relations before syncing the data directory?

> On Oct 10, 2026, at 11:44, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Sat, 10 Oct 2026 at 07:36, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>>
>>
>>> On Oct 8, 2026, at 05:20, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)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/