Re: Return pg_control from pg_backup_stop().

From: shihao zhong <zhong950419(at)gmail(dot)com>
To: David Steele <david(at)pgbackrest(dot)org>
Cc: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Andy Pogrebnoi <andrew(dot)pogrebnoi(at)percona(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Haibo Yan <tristan(dot)yim(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Shaun Thomas <shaun(dot)thomas(at)pgedge(dot)com>
Subject: Re: Return pg_control from pg_backup_stop().
Date: 2026-09-16 04:14:57
Message-ID: CAGRkXqQ737wzDadAh3qQncZm-S8QBEPu5NdiK2u_c8DDHsj=9g@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi David, Andrey,

Tested v13 on master (04c4c1c3a96). recovery, pg_rewind, pg_basebackup and
regress all pass. Two comments.

1. The pg_resetwal sentence in pg_basebackup.sgml

I asked for the field to be documented and still think it should be, but the
wording offers pg_resetwal as a way out and it is not one. It clears the
flag
and resets the WAL, but leaves backup_label, so the cluster still does not
start. The flag is gone by then, so the hint v13 just conditioned away is
back, and the server itself says "try removing the file backup_label". They
do, and it starts silently inconsistent. Unpatched master behaves
identically,
what is new is that users get sent there. Say what clears the flag, but make
restoring backup_label the remedy. Also rename the "pg_resetwal clears the
flag" subtest in 057.
Separately, pg_resetwal never mentions backup_label in code or docs, and
always leaves an unstartable cluster when one is present. A warning there
would cut this off at the first step. Happy to post that as its own patch.

2. The assert in backup_control_file()

Michael, this one is for you since David left the call to you. Assert builds
only, so no production impact, but the invariant is false and I would drop
the block rather than narrow it. Only xlog_redo() for XLOG_CHECKPOINT_ONLINE
updates the struct without calling UpdateControlFile, so
checkPointCopy.nextXid
is the only field that can differ, and InitWalRecovery replaces
checkPointCopy
wholesale when backup_label is present. The copy we ship is correct either
way, and asserting it equals the file contradicts the premise that shared
memory is authoritative. One detail on Andrey's recipe: a checkpoint
replayed
before the backup never trips it, since do_pg_backup_start forces a
restartpoint. Only replay during file sending matters.

Otherwise patch 01 looks ready to me.

Thanks,
Shihao

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nisha Moond 2026-09-16 04:20:27 Re: Distinguish publication exclusions in object addresses
Previous Message shveta malik 2026-09-16 04:12:52 Re: Distinguish publication exclusions in object addresses