Re: Fix handling of copy_file_range() return value

Yingying Chen <cyy9255@gmail.com>

From: Yingying Chen <cyy9255@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2026-06-22T08:46:45Z
Lists: pgsql-hackers
On Mon, Jun 22, 2026 at 3:20 PM Peter Eisentraut <peter@eisentraut.org>
wrote:

> While checking return/error handling of file system calls, I found that
> the copy_file_range() call in pg_combinebackup has a potential problem.
> If copy_file_range() returns 0, which is a documented condition, then
> the loop never makes progress and could spin forever.
>
> The other uses of copy_file_range() in the tree are surrounded by
> different logic and don't appear to have this problem.
>
> My suggested fix is to make a return value of 0 an error.  It most
> likely indicates that the source file has an unexpected size.
>
Hi, Peter,

Thanks for the patch. I agree wb==0 should be treated as an error, so the
fix direction looks good to me.

I have a small comment:
====
        wb = copy_file_range(s->fd, &off, wfd, NULL, BLCKSZ - nwritten, 0);

        if (wb < 0)
                pg_fatal("error while copying file range from \"%s\" to
\"%s\": %m",
                                 s->filename, output_filename);
        else if (wb == 0)
                pg_fatal("unexpected end of file while copying file range
from \"%s\" to \"%s\"",
                                 s->filename, output_filename);


As copy_file_range copies from s->fd, should pg_fatal just s->filename in
the error message? In that case, input_filename is no longer used in
write_reconstructed_file(), then it might be removed from the argument list.

Regards,
Yingying Chen

Commits

  1. Fix handling of copy_file_range() return value

  2. Allow using copy_file_range in write_reconstructed_file

  3. Add support for incremental backup.