On 11/20/23 14:44, Robert Haas wrote:
On Mon, Nov 20, 2023 at 12:54 PM David Steele <da...@pgmasters.net> wrote:
Another thing we could do is explicitly error if we see backup_label in
PGDATA during recovery. That's just a few lines of code so would not be
a big deal to maintain. This error would only be visible on restore, so
it presumes that backup software is being tested.
I think that if we do decide to adopt this proposal, that would be a
smart precaution.
I'd be OK with it -- what do you think, Michael? Would this be enough
that we would not need to rename the functions, or should we just go
with the rename?
A little hard to add to the tests, I think, since they are purely
informational, i.e. not pushed up by the parser. Maybe we could just
grep for the fields?
Hmm. Or should they be pushed up by the parser?
We could do that. I started on that road, but it's a lot of code for
fields that aren't used. I think it would be better if the parser also
loaded a data structure that represented the manifest. Seems to me
there's a lot of duplicated code between pg_verifybackup and
pg_combinebackup the way it is now.
I think these fields would be handled the same as the rest of the fields
in backup_label: returned from pg_backup_stop() and also stored in
backup_manifest. Third-party software can do as they like with them and
pg_combinebackup can just read from backup_manifest.
I think that would be a bad plan, because this is critical
information, and a backup manifest is not a thing that you're required
to have. It's not a natural fit at all. We don't want to create a
situation where if you nuke the backup_manifest then the server
forgets that what it has is an incremental backup rather than a usable
data directory.
I can't see why a backup would continue to be valid without a manifest
-- that's not very standard for backup software. If you have the
critical info in backup_label, you can't afford to lose that, so why
should backup_manifest be any different?
Regards,
-David
