On 28/05/2026 09:01, Ayush Tiwari wrote:
> Hi,
>
> While looking at pg_rewind's source directory traversal, I noticed a small
> thing in recurse_dir() that I wanted to ask about.
>
> When lstat() fails with ENOENT, the existing comment says that this is OK
> when scanning a live source data directory, because the file may have been
> removed by the new primary:
>
> File doesn't exist anymore. This is ok, if the new primary
> is running and the file was just removed.
>
> But after that branch, the code continues and later checks fst.st_mode to
> decide whether the entry is a regular file, directory, or symlink. Since
> lstat() failed in that case, is fst still safe to inspect? It seems to me
> that the intended behavior might be to skip that directory entry once ENOENT
> is accepted.
>
> The attached patch adds a continue in that branch.
>
> I tested this with a small harness around traverse_datadir() and an lstat()
> shim that returns ENOENT for one directory entry. Without the patch, the
> callback was still invoked for that entry; with the patch, it was skipped.
>
> 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.
- Heikki
