Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 29 Sep 2023 11:30:21 +0900
From: Michael Paquier <michael@paquier.xyz>
To: David Steele <david@pgmasters.net>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: The danger of deleting backup_label
Message-ID: <ZRY2vdlKJqhuZqGs@paquier.xyz>
References: <1330cb48-4e47-03ca-f2fb-b144b49514d8@pgmasters.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="KCSWMwjVzCvxOGbk"
Content-Disposition: inline
In-Reply-To: <1330cb48-4e47-03ca-f2fb-b144b49514d8@pgmasters.net>
Archived-At: <https://www.postgresql.org/message-id/ZRY2vdlKJqhuZqGs%40paquier.xyz>
Precedence: bulk


--KCSWMwjVzCvxOGbk
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Sep 28, 2023 at 05:14:22PM -0400, David Steele wrote:
> While reading through [1] I saw there were two instances where backup_lab=
el
> was removed to achieve a "successful" restore. This might work on trivial
> test restores but is an invitation to (silent) disaster in a production
> environment where the checkpoint stored in backup_label is almost certain=
 to
> be earlier than the one stored in pg_control.

Definitely successful.

> Recovery worked perfectly as long as backup_label was present and failed
> hard when it was not:
>=20
> LOG:  invalid primary checkpoint record
> PANIC:  could not locate a valid checkpoint record
>=20
> It's not a very good message, but at least the foot gun has been removed.=
 We
> could use this as a special value to give a better message, and maybe use
> something a bit more unique like 0xFFFFFFFFFADEFADE (or whatever) as the
> value.

Why not just InvalidXLogRecPtr?

> This is all easy enough for pg_basebackup to do, but will certainly be
> non-trivial for most backup software to implement. In [2] we have discuss=
ed
> perhaps returning pg_control from pg_backup_stop() for the backup software
> to save, or it could become part of the backup_label (encoded as hex or
> base64, presumably). I prefer the latter as this means less work for the
> backup software (except for the need to exclude pg_control from the backu=
p).
>=20
> I don't have a patch for this yet because I did not test this idea using
> pg_basebackup, but I'll be happy to work up a patch if there is interest.

If the contents of the control file are tweaked before sending it
through a BASE_BACKUP, it would cover more than just pg_basebackup.
Switching the way the control file is sent with new contents in
sendFileWithContent() rather than sendFile() would be one way, for
instance..
--
Michael

--KCSWMwjVzCvxOGbk
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmUWNr0ACgkQnvQgOdby
QH18QA//TFWABdPVqId86fOe3lLqshCwndQ5UFkFkS1u615lHiKeDDRU2dx+uxmE
E1Hz9/JkGD/DAd+KT9foqV+xPtGMYyqogYzDoyk/pmhS4ixnONLflXvUUOSzExo2
lfmYGMZkuDs32NWmQRFLMYaR2DVZZEzDV7KV1zcw1sIjuJgRo2oT3HenuGRj1NvT
iCyYTUH75RTZM5B70fS4jrMwMxbuppqH3htiyD51YH/9SUAXehulzCQaxUeKjlPv
1Uqck9mNmGvkCwzxj6+9FkS3SPuJXTIrYAjtRSitfYCHAplyfE8R2Dxe2c7npQgi
n53ECbPeZcP/CIS8BXwS52cAWfenCOce2wY3vyc1VKkbAO9uSYxJn+TrPGuVPMiK
SAjTLrmeJtKqGhXZKF4kgxuJQqHTUDUtzOVVE8qFQ384FC4pIB23DCtw+ZTz5kPs
UTR3DCD3rJYOzLkpl8hsd/wvD9k853VE5M77N0MLLdFT1XGHAeyGCSXO0cgmzNE8
H8GKdeQH+FyokATv24MIetnWN1qIuEoflLRDIgppgYnPOua6BaZ6CVdWGa6YRFJZ
aIgPCnJRPwgTzMzQ29Gfys6hB/0U49TeMEInfZr/Bqakfx6vCTE7vGp37c+ziKQq
Com3d2fKRqWafuRFYqBO4/hD7qTga8TUsPV4yuR8MLoL2unxHs4=
=9R2H
-----END PGP SIGNATURE-----

--KCSWMwjVzCvxOGbk--