Re: backup manifests

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>
Cc: Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Suraj Kharage <suraj(dot)kharage(at)enterprisedb(dot)com>, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, Tels <nospam-pg-abuse(at)bloodgate(dot)com>, David Steele <david(at)pgmasters(dot)net>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>
Subject: Re: backup manifests
Date: 2020-03-05 16:55:39
Message-ID: CA+TgmoYJxdwPSd2as1z5+W40hxuhrnKdgiN+4YY0ppmTC-r36Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 5, 2020 at 7:05 AM tushar <tushar(dot)ahuja(at)enterprisedb(dot)com> wrote:
> There is one small observation if we use slash (/) with option -i then not getting the desired result

Here's an updated patch set responding to many of the comments
received thus far. Since there are quite a few emails, let me
consolidate my comments and responses here.

Report: Segmentation fault if -m is used to point to a valid manifest,
but actual backup directory is nonexistent.
Response: Fixed; thanks for the report.

Report: pg_validatebackup doesn't complain about problems within the
pg_wal directory.
Response: That's out of scope. The WAL files are fetched separately
and are therefore not part of the manifest.

Report: Inaccessible file in data directory being validated leads to a
double free.
Response: Fixed; thanks for the report.

Report: Patch 0005 doesn't validate the manifest checksum.
Response: I know. I mentioned that when posting the previous patch
set. Fixed in this version, though.

Report: Removing an empty directory doesn't make backup validation
fail, even though it might cause problems for the server.
Response: That's a little unfortunate, but I'm not sure it's really
worth complicating the patch to deal with it. It's something of a
corner case.

Report: Negative file sizes in the backup manifest are interpreted as
large integers.
Response: That's also a little unfortunate, but I doubt it's worth
adding code to catch it, since any such manifest is corrupt. Also,
it's not like we're ignoring it; the error just isn't ideal.

Report: If I take the backup label from backup #1 and stick it into
otherwise-identical backup #2, validation succeeds but the server
won't start.
Response: That's because we can't validate the pg_wal directory. As
noted above, that's out of scope.

Report: Using --ignore with a slash-terminated pathname doesn't work
as expected.
Response: Fixed, thanks for the report.

Off-List Report: You forgot a PG_BINARY flag.
Response: Fixed. I thought I'd done this before but there were two
places and I'd only fixed one of them.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
v9-0001-Add-checksum-helper-functions.patch application/octet-stream 9.8 KB
v9-0004-Modify-server-code-to-generate-backup-manifest-in.patch application/octet-stream 8.2 KB
v9-0003-pg_validatebackup-Validate-a-backup-against-the-b.patch application/octet-stream 35.0 KB
v9-0002-Generate-backup-manifests-for-base-backups.patch application/octet-stream 33.4 KB
v9-0005-Adjust-pg_validate-to-validate-a-JSON-format-mani.patch application/octet-stream 24.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2020-03-05 16:57:07 Re: reindex concurrently and two toast indexes
Previous Message Alexey Kondratov 2020-03-05 16:52:24 Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line