| From: | Joao Detomini <joao(dot)detomini(at)enterprisedb(dot)com> |
|---|---|
| To: | David Steele <david(at)pgbackrest(dot)org> |
| Cc: | shihao zhong <zhong950419(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: pg_resetwal: refuse to run when backup_label exists |
| Date: | 2026-10-05 03:10:06 |
| Message-ID: | CABH8dKwKVRRLB2vLmL+FARmYbAaqYbOJkFsasPMzRH1hZiiynA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Shihao,
I applied v2-0001 and v2-0002 on current master and ran the
pg_resetwal tests, plus the recovery and pg_basebackup suites since
pg_createsubscriber also runs pg_resetwal; no failures. I also tried
it by hand on a pg_basebackup copy: both "pg_resetwal -f" and
"pg_resetwal -n" now refuse with the new error and hint.
I checked the claim about the order of operations as well. On
unpatched master, "pg_resetwal -f" on a backup makes the server fail
to start ("could not locate required checkpoint record"), and the
server's own hint says that removing backup_label leaves a corrupt
cluster. I then took two copies of the same backup, one with
pg_resetwal first and backup_label removed afterwards, the other the
other way round. The pg_controldata output and the pg_wal file names
are the same, except for the timestamps.
Given that, I'd keep the check as it is, without letting -f override
it. The server won't start until backup_label is gone anyway, so
asking for the removal first doesn't take anything away from the
salvage case Michael described, it only makes the user do it on
purpose. Today the warning about backup_label only shows up after
pg_resetwal has already run, and with -f nothing warns at all.
Two small things. The Discussion: trailer points at David's message
in the CF 4997 thread; I guess it should point at this thread. And a
restored backup normally has tablespace_map too, while the check only
looks at backup_label; is ignoring it on purpose?
Thanks,
João Marcelo
Em sex., 2 de out. de 2026 às 07:25, David Steele <david(at)pgbackrest(dot)org>
escreveu:
> On 10/2/26 07:43, shihao zhong wrote:
> > Hi Michael,
> > > However, if
> > > one knows his business, even using pg_resetwal on a data folder with a
> > > backup_label file around can prove incredibly useful when salvaging
> > > data from a corrupted instance.
> >
> > That still works with the patch. The only change is that backup_label
> > has to be removed before pg_resetwal and not after. Today it has to go
> > anyway. After pg_resetwal -f the server stops with "could not locate
> > required checkpoint record" until the file is removed. Both orders give
> > the same data directory on master, pg_control and the new WAL segment
> > included.
> >
> > Does that change your view? If not, I can let -f override the check.
> > Then the patch is only a clearer error without -f, plus the doc
> > paragraph.
>
> Given that the server will not start after pg_resetwal until
> backup_label is removed I think it makes sense to force the user to
> remove it beforehand. I'd prefer they remove it manually but I suppose
> we could have -f remove it. Either way, it seems like pg_resetwal should
> leave the cluster in a state where it can be started.
>
> > > One thing that may be interesting to me is something much different
> > > than what you are sending: an option to overwrite DBState in
> > > ControlFileData to something else than DB_SHUTDOWNED.
> >
> > 0003 in the attached v2 is a first try for that. It adds
> > --cluster-state, which takes shut-down, shut-down-in-recovery,
> > shutting-down, in-crash-recovery, in-archive-recovery or in-production.
> > DB_STARTUP is left out because the server refuses to start with it.
> >
> > With any value other than shut-down, the next start goes through crash
> > recovery. One visible effect is that unlogged tables are emptied. Today
> > they keep what was on disk after a crash and pg_resetwal -f.
> I'd say this should be the subject of a separate patch and thread. I'm
> honestly not sure how useful it would be aside from test scenarios.
>
> Regards,
> -David
>
>
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Koshi Shibagaki (Fujitsu) | 2026-10-05 03:23:37 | [PATCH] pg_walsummary: suppress limit output with --quiet |
| Previous Message | David Rowley | 2026-10-05 03:00:59 | Re: Table Function Scan can report incorrect "Maximum Storage" in EXPLAIN |