Horton

Re: backup manifests

Andres Freund <andres@anarazel.de>

View thread
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: David Steele <david@pgmasters.net>, Noah Misch <noah@leadboat.com>, Stephen Frost <sfrost@snowman.net>, 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>, 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-04-02T17:23:18Z
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.

Hi,

On 2020-04-02 13:04:45 -0400, Robert Haas wrote:
> And here's another new patch set. After some experimentation, I was
> able to manually test the timeline-switch-during-a-base-backup case
> and found that it had bugs in both pg_validatebackup and the code I
> added to the backend's basebackup.c. So I fixed those.

Cool.


> It would be
> nice to have automated tests, but you need a large database (so that
> backing it up takes non-trivial time) and a load on the primary (so
> that WAL is being replayed during the backup) and there's a race
> condition (because the backup has to not finish before the cascading
> standby learns that the upstream has been promoted), so I don't at
> present see a practical way to automate that. I did verify, in manual
> testing, that a problem with WAL files on either timeline caused a
> validation failure. I also verified that the LSNs at which the standby
> began replay and reached consistency matched what was stored in the
> manifest.

I suspect its possible to control the timing by preventing the
checkpoint at the end of recovery from completing within a relevant
timeframe. I think configuring a large checkpoint_timeout and using a
non-fast base backup ought to do the trick. The state can be advanced by
separately triggering an immediate checkpoint? Or by changing the
checkpoint_timeout?



> I also implemented Noah's suggestion that we should write the backup
> manifest under a temporary name and then rename it afterward.
> Stephen's original complaint that you could end up with a backup that
> validates successfully even though we died before we got the WAL is,
> at this point, moot, because pg_validatebackup is now capable of
> noticing that the WAL is missing. Nevertheless, this seems like a nice
> belt-and-suspenders check.

Yea, it's imo generally a good idea.


> I think this responds to pretty much all of the complaints that I know
> about and upon which we have a reasonable degree of consensus. There
> are still some things that not everybody is happy about. In
> particular, Stephen and David are unhappy about using CRC-32C as the
> default algorithm, but Andres and Noah both think it's a reasonable
> choice, even if not as robust as everybody will want. As I agree, I'm
> going to stick with that choice.

I think it might be worth looking, in a later release, at something like
blake3 for a fast cryptographic checksum. By allowing for instruction
parallelism (by independently checksuming different blocks in data, and
only advancing the "shared" checksum separately) it achieves
considerably higher throughput rates.

I suspect we should also look at a better non-crypto hash. xxhash or
whatever. Not just for these checksums, but also for in-memory.


> Also, there is still some debate about what the tool ought to be
> called. My previous suggestion to rename this from pg_validatebackup
> to pg_validatemanifest seems wrong now that WAL validation has been
> added; in fact, given that we now have two independent sanity checks
> on a backup, I'm going to argue that it would be reasonable to extend
> that by adding more kinds of backup validation, perhaps even including
> the permissions check that Andres suggested before.

FWIW, the only check I'd really like to see in this release is the
crosscheck with the files length and the actually read data (to be able
to disagnose FS issues).


Greetings,

Andres Freund