Re: pg_rewind: Skip vanished source files during traversal

Hi all,

On Mon, Sep 7, 2026 at 3:35 PM Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
>
> Hi.
>
> On Mon, 6 Jul 2026 at 18:49, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>
>> On 28/05/2026 09:01, Ayush Tiwari wrote:
>> >
>> > Does this interpretation make sense, or is there a reason we should continue
>> > processing the entry after ENOENT?
>>
>> So, the full comment is:
>>
>> > /*
>> > * File doesn't exist anymore. This is ok, if the new primary
>> > * is running and the file was just removed. If it was a data
>> > * file, there should be a WAL record of the removal. If it
>> > * was something else, it couldn't have been anyway.
>> > *
>> > * TODO: But complain if we're processing the target dir!
>>
>> That explanation doesn't make a whole lot of sense to me. We don't
>> support running pg_rewind on a source directory while the source server
>> is running. We do support the "connection" mode on a running server, but
>> that doesn't use this function. (And it's not clear what the "couldn't
>> have been anyway" means here. Mea culpa, I wrote that comment)
>>
>>
>> And it would indeed be nice to implement that TODO. But AFAICS we should
>> just always throw an error here. It's not OK if a file goes missing in
>> the target dir, and we don't expect it in the source dir either, because
>> the server shouldn't be running.
>
>
> Ahh, yes you are right. I'd only been looking at the uninitialized
> fst.st_mode read (lstat() fails, but the code still inspects st_mode),
> and continue was the minimal way to avoid it.
>
> In supported paths, recurse_dir() only runs on a stopped local source
> data directory or the target data directory; the running-source case
> goes through libpq_traverse_files(). So a vanished file isn't expected
> there, and erroring would be the better fix.
>
> v2 attached: it just pg_fatal()s on lstat() failure, dropping the ENOENT
> special case and the TODO.
>

Thank you for the updated patch. I reviewed and tested the v2 patch.
The patch changes the handling of lstat() failures in recurse_dir()
and instead of ignoring an ENOENT and continuing the traversal, the
patch now reports the failure as a fatal error. This looks correct to
me since recurse_dir() is used for stopped data directories, while the
running source-server case uses a different traversal path. I also
tested the specific failure scenario by creating a real timeline
divergence between the source and target clusters. I used an
LD_PRELOAD shim to force lstat() to return ENOENT for a selected file
under the source global directory.
With the patch, pg_rewind correctly stopped with:
pg_rewind: error: could not stat file
"/tmp/pg_rewind_test/source/global/vanishme2": No such file or
directory
and exited with status 1
I also verified that the target cluster was not modified after the
failed rewind. The existing target-side table was still present, while
the source-only table was not introduced. Based on the above testing,
the patch behaves as expected and correctly handles an unexpectedly
vanished file during traversal.
The patch looks good to me.

Regards
Solai