Re: backup manifests

Stephen Frost <sfrost@snowman.net>

From: Stephen Frost <sfrost@snowman.net>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Suraj Kharage <suraj.kharage@enterprisedb.com>, tushar <tushar.ahuja@enterprisedb.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Rushabh Lathia <rushabh.lathia@gmail.com>, Tels <nospam-pg-abuse@bloodgate.com>, David Steele <david@pgmasters.net>, Andrew Dunstan <andrew.dunstan@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Jeevan Chalke <jeevan.chalke@enterprisedb.com>, vignesh C <vignesh21@gmail.com>
Date: 2020-03-27T15:26:56Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Try to avoid compiler warnings in optimized builds.

  2. Fix option related issues in pg_verifybackup.

  3. Add index term for backup manifest in documentation.

  4. Code review for backup manifest.

  5. Document the backup manifest file format.

  6. Fix typo in pg_validatebackup documentation.

  7. Exclude backup_manifest file that existed in database, from BASE_BACKUP.

  8. Msys2 tweaks for pg_validatebackup corruption test

  9. Fix resource management bug with replication=database.

  10. Be more careful about time_t vs. pg_time_t in basebackup.c.

  11. pg_validatebackup: Fix 'make clean' to remove tmp_check.

  12. pg_validatebackup: Also use perl2host in TAP tests.

  13. Generate backup manifests for base backups, and validate them.

  14. Add checksum helper functions.

  15. pg_waldump: Add a --quiet option.

  16. Catversion bump for b9b408c48724

  17. pg_basebackup: Refactor code for reading COPY and tar data.

  18. Use a ResourceOwner to track buffer pins in all cases.

  19. Use ARMv8 CRC instructions where available.

  20. Logical replication support for initial data copy

  21. Use Intel SSE 4.2 CRC instructions where available.

  22. Switch to CRC-32C in WAL and other places.

  23. Remove support for 64-bit CRC.

  24. Change CRCs in WAL records from 64bit to 32bit for performance reasons.

Greetings,

* Andres Freund (andres@anarazel.de) wrote:
> On 2020-03-26 11:37:48 -0400, Robert Haas wrote:
> > I'm sorry that you can't see how that's sensible, but it doesn't mean
> > that it isn't sensible. It is totally unrealistic to expect that any
> > backup verification tool can verify that you won't get an error when
> > trying to use the backup. That would require that everything that the
> > validation tool try to do everything that PostgreSQL will try to do
> > when the backup is used, including running recovery and updating the
> > data files. Anything less than that creates a real possibility that
> > the backup will verify good but fail when used. This tool has a much
> > narrower purpose, which is to try to verify that we (still) have the
> > files the server sent as part of the backup and that, to the best of
> > our ability to detect such things, they have not been modified. As you
> > know, or should know, the WAL files are not sent as part of the
> > backup, and so are not verified. Other things that would also be
> > useful to check are also not verified. It would be fantastic to have
> > more verification tools in the future, but it is difficult to see why
> > anyone would bother trying if an attempt to get the first one
> > committed gets blocked because it does not yet do everything. Very few
> > patches try to do everything, and those that do usually get blocked
> > because, by trying to do too much, they get some of it badly wrong.
> 
> It sounds to me that if there are to be manifests for the WAL, it should
> be a separate (set of) manifests. Trying to somehow tie together the
> manifest for the base backup, and the one for the WAL, makes little
> sense to me. They're commonly not computed in one place, often not even
> stored in the same place. For PITR relevant WAL doesn't even exist yet
> at the time the manifest is created (and thus obviously cannot be
> included in the base backup manifest). And fairly obviously one would
> want to be able to verify the correctness of WAL between two
> basebackups.

We aren't talking about generic PITR or about tools other than
pg_basebackup, which has specific options for grabbing the WAL, and
making sure that it is all there for the backup that was taken.

> I don't see much point in complicating the design to somehow capture WAL
> in the manifest, when it's only going to solve a small set of cases.

As it relates to this, I tend to think that it solves the exact case
that pg_basebackup is built for and used for.  I said up-thread that if
someone does decide to use -X none then we could just throw a warning
(and perhaps have a way to override that if there's desire for it).

> Seems better to (later?) add support for generating manifests for WAL
> files, and then have a tool that can verify all the manifests required
> to restore a base backup.

I'm not trying to expand on the feature set here or move the goalposts
way down the road, which is what seems to be what's being suggested
here.  To be clear, I don't have any objection to adding a generic tool
for validating WAL as you're talking about here, but I also don't think
that's required for pg_validatebackup.  What I do think we need is a
check of the WAL that's fetched when people use pg_basebackup -Xstream
or -Xfetch.  pg_basebackup itself has that check because it's critical
to the backup being successful and valid.  Not having that basic
validation of a backup really just isn't ok- there's a reason
pg_basebackup has that check.

Thanks,

Stephen