pg_resetwal: refuse to run when backup_label exists

On Mon, Sep 28, 2026 at 11:02:00PM -0700, shihao zhong wrote:

Robert worried in [3] that people with a bad backup will just run
pg_resetwal, which is worse than starting from the wrong checkpoint. This
patch targets that step. A backup that still has its backup_label is the
case where the user has everything needed for a correct restore, and
pg_resetwal is the wrong tool. Now it says so before it does any damage. A
user can still delete the file and rerun, but that is a second deliberate
step, and the docs now say when that is safe.

0002 adds TAP tests and is optional. I think this is master only, since it
changes what an existing command accepts.

FWIW, I don't think that we should restrict that at all. pg_resetwal
is a footgun if one does not know what he/she is doing. 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.

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. Hence that
would be the opposite of a restriction.

Note: pg_resetwal should have been renamed a long time ago. Perhaps
it should just be pg_control_update or something like that. I am
pretty sure that pg_footgun has been mentioned to me once, at some
point. Jokes apart, *that* naming could be a serious option to make
people aware that this a tool you should not use if you do not
absolutely know what you are doing.
--
Michael

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.

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.

Thanks,
Shihao

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

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@pgbackrest.org>
escreveu:

Hi Joao,

T hanks for testing it, and for checking the order of operations.

The Discussion: trailer points at David's message
in the CF 4997 thread; I guess it should point at this thread.

The patch came out of that thread. That message is where David said
-f should not override the check. v3 keeps it and adds this thread as
a second Discussion line.

And a
restored backup normally has tablespace_map too, while the check only
looks at backup_label; is ignoring it on purpose?

Yes. The server reads tablespace_map only when backup_label is there.
Without backup_label it renames the file to tablespace_map.old and
starts as usual. So tablespace_map alone does not stop the server from
starting, and there is nothing for pg_resetwal to guard against.
v3 also leaves out the --cluster-state patches, as David suggested.
0001 and 0002 have no code changes.

Thanks,
Shihao

Hi Shihao,

Thanks for v3. I applied it on current master and the pg_resetwal
tests pass.

On -f: pg_resetwal only lets it override uncertainty about the state
of pg_control, not known facts about the directory. postmaster.pid is
the closest case. -f doesn't bypass it and the user has to delete the
file by hand, even though a stale one after a crash is normal. A
backup_label looks like that second kind to me, so I'd keep v3 as is.

Thanks,
João Marcelo

Em seg., 5 de out. de 2026 às 00:47, shihao zhong <zhong950419@gmail.com>
escreveu:

Hi Joao,

Thanks for testing v3.

A backup_label looks like that second kind to me, so I'd keep v3
as is.

Agreed, I will leave v3 as it is.

I see the CF entry is now Waiting on Author. Is there something you
want me to change in v3? If not, I think it can go back to Needs
review, or Ready for Committer if you think it is ready.

Thanks,
Shihao

Hi Shihao,

Nothing to change from my side, v3 looks fine to me. I'll put it
back to Needs review for now, since Michael hasn't said whether he's
ok with -f not overriding the check.

Thanks,
João Marcelo

Em seg., 5 de out. de 2026 às 11:56, shihao zhong <zhong950419@gmail.com>
escreveu:

On Mon, Oct 05, 2026 at 12:04:14PM -0300, Joao Detomini wrote:

Nothing to change from my side, v3 looks fine to me. I'll put it
back to Needs review for now, since Michael hasn't said whether he's
ok with -f not overriding the check.

I'm maybe just not seeing the value of the patch, then, that's all, so
I doubt that my opinion has any value.

Please note that the community mailing lists follow the following
style in terms of message posting:
https://en.wikipedia.org/wiki/Posting_style#Bottom-posting
--
Michael

On Mon, Oct 05, 2026 at 07:32 PM, Michael Paquier wrote:

I'm maybe just not seeing the value of the patch, then, that's all, so
I doubt that my opinion has any value.

That's fine, thanks for taking a look.

Please note that the community mailing lists follow the following
style in terms of message posting:

Noted, sorry about that.

Shihao, I'm moving the entry to Ready for Committer. The code and
tests look good to me, and the question of whether -f should be able
to override the check is left for the committer to decide.

Regards,
João Marcelo

Em seg., 5 de out. de 2026 às 19:32, Michael Paquier <michael@paquier.xyz>
escreveu:

Hi Shihao,
I have a local test result for this patch on macOS 26.6.2 (arm64).
My checkout contains v2-0001 (code/docs) and v3-0002 (tests), on base
e01fc6315f19a794301ca8889d37ae629862e2ac.

This report is for that combination, not a claim to have tested a newer
complete series. T
he build succeeded with --without-icu --enable-tap-tests. A
fter installing to a private prefix,
I ran: make -C src/bin/pg_resetwal installcheck PROVE_FLAGS=--verbose Both
runs passed: Files=2, Tests=109, Result: PASS.
The added tests cover refusal with backup_label present, including --force
and --dry-run, and successful reset and server startup after removing the
synthetic label.
One possible test enhancement would be to assert that pg_control, the WAL
files, and backup_label remain unchanged after each refusal.
Regards, Tinh

Vào Thứ 7, 10 thg 10, 2026 vào lúc 20:20 shihao zhong <
zhong950419@gmail.com> đã viết:

i Tinh,

Thanks for testing.

One possible test enhancement would be to assert that pg_control,
the WAL files, and backup_label remain unchanged after each refusal.

I would leave that out. The check runs before pg_resetwal reads
pg_control, right after the postmaster.pid check, so nothing has been
written at that point. The postmaster.pid refusal has no such test
either. I did check by hand, and all three are unchanged after each
refusal.

Thanks,
Shihao

Hi Shihao,

Thanks for checking this manually. That makes
sense—the early check and consistency with the postmaster.pid

I have no further comments on this point.

Regards,
Tinh

Vào CN, 11 thg 10, 2026 vào lúc 09:32 shihao zhong <zhong950419@gmail.com>
đã viết: