Horton

Thread

Commits

  1. Fix version number calculation for data folder flush in pg_combinebackup

  1. Incorrect version number given to sync_pgdata() in pg_combinebackup.c

    Michael Paquier <michael@paquier.xyz> — 2025-10-10T06:21:25Z

    Hi all,

While hacking on a different thing that touched pg_combinebackup, I
have bumped into a silly bug.

To keep it short, the version number is calculated based on this code
in read_pg_version_file(), where "version" is the result of strtoul() 
applied to the contents of PG_VERSION:
    return version * 10000;

For v18, this would return 180000, which is fine.

A bit later on, we do that, which is not fine:
    sync_pgdata(opt.output, version * 10000, opt.sync_method, true);

This leads to a version number higher than expected, multiplied twice.
This was harmless, because sync_pgdata uses the version number to make
the difference between pg_wal/ and pg_xlog/, and pg_combinebackup does
not support versions older than v10, which is exactly where the
renamed happened.  Hence, even if the version number was too high, we
always expect to flush pg_wal/.

Trivial patch attached, for a backpatch down to where pg_combinebackup
has been introduced.

Thoughts?
--
Michael

  2. Re: Incorrect version number given to sync_pgdata() in pg_combinebackup.c

    Chao Li <li.evan.chao@gmail.com> — 2025-10-10T06:39:58Z

    > On Oct 10, 2025, at 14:21, Michael Paquier <michael@paquier.xyz> wrote:
> 
> Hi all,
> 
> While hacking on a different thing that touched pg_combinebackup, I
> have bumped into a silly bug.
> 
> To keep it short, the version number is calculated based on this code
> in read_pg_version_file(), where "version" is the result of strtoul() 
> applied to the contents of PG_VERSION:
>    return version * 10000;
> 
> For v18, this would return 180000, which is fine.
> 
> A bit later on, we do that, which is not fine:
>    sync_pgdata(opt.output, version * 10000, opt.sync_method, true);
> 
> This leads to a version number higher than expected, multiplied twice.
> This was harmless, because sync_pgdata uses the version number to make
> the difference between pg_wal/ and pg_xlog/, and pg_combinebackup does
> not support versions older than v10, which is exactly where the
> renamed happened.  Hence, even if the version number was too high, we
> always expect to flush pg_wal/.
> 
> Trivial patch attached, for a backpatch down to where pg_combinebackup
> has been introduced.
> 
> Thoughts?
> --
> Michael
> <pg_combinebackup-version.patch>


Yeah, looks like a stupid bug. read_pg_version_file() has multiplied 10000 to version number.

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

  3. Re: Incorrect version number given to sync_pgdata() in pg_combinebackup.c

    Michael Paquier <michael@paquier.xyz> — 2025-10-13T23:40:45Z

    On Fri, Oct 10, 2025 at 02:39:58PM +0800, Chao Li wrote:
> Yeah, looks like a stupid bug. read_pg_version_file() has multiplied
> 10000 to version number.

Thanks for the review.  Applied this one down to v17.
--
Michael