Re: trying again to get incremental backup

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: trying again to get incremental backup
Date: 2023-11-30 14:33:04
Message-ID: CA+Tgmob7CFCv99_bCRB=pWYat6r2rhgSqiEWQPJO67ywcb0Hpw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

New patch set.

0001: Rename JsonManifestParseContext callbacks, per feedback from
Álvaro. Not logically related to the rest of this, except by code
proximity. Separately committable, if nobody objects.

0002: Rename record_manifest_details_for_{file,wal_range}, per
feedback from Álvaro that the names were too generic. Separately
committable, if nobody objects.

0003: Move parse_manifest.c to src/common. No significant changes
since the previous version.

0004: Add a new WAL summarizer process. No significant changes since
the previous version.

0005: Incremental backup itself. Changes:
- Remove UPLOAD_MANIFEST replication command and instead add
INCREMENTAL_WAL_RANGE replication command.
- In consequence, load_manifest.c which was included in the previous
patch sets now moves to src/fe_utils and has some adjustments.
- Actually document the new replication command which I overlooked previously.
- Error out promptly if an incremental backup is attended with
summarize_wal = off.
- Fix test in copy_file(). We should be willing to use the fast-path
if a new checksum is *not* required, but the sense of the test was
inverted in previous versions.
- Fix some handling of the missing-manifest case in pg_combinebackup.
- Fix terminology in a help message.

0006: Add pg_walsummary tool. No significant changes since the previous version.

0007: Test patch, not for commit.

As far as I know, the main commit-blockers here are (1) the timeout
when waiting for WAL summarization is still hard-coded to 60 seconds
and (2) the ubsan issue that Thomas hunted down, which would cause at
least the entire CF environment and maybe some portion of the BF to
turn red if this were committed. That issue is in xlogreader rather
than in this patch set, at least in part, but it still needs fixing
before this goes ahead. I also suspect that the slightly-more
significant refactoring in this version may turn up a few new bugs in
the CF environment. I think once that the aforementioned items are
sorted out, this could be committed through 0005, and 0001 and 0002
could be committed sooner. 0006 should have some tests written before
it gets committed, but it doesn't necessarily have to be committed at
the exact same moment as everything else, and writing tests isn't that
hard, either.

Other loose ends that would be nice to tidy up at some point:

- Incremental JSON parsing so we can handle huge manifests.

- More documentation as proposed by Álvaro but I'm failing to find the
details of his proposal right now.

- More introspection facilities, maybe, or possibly rip some some
stuff out of WalSummarizerCtl if we don't want it. This one might be a
higher priority to address before initial commit, but it's probably
not absolutely critical, either.

I'm not quite sure how aggressively to press forward with getting
stuff committed. I'd certainly rather debug as much as I can locally
and via cfbot before turning the buildfarm pretty colors, but I think
it generally works out better when larger features get pushed earlier
in the cycle rather than in the mad frenzy right before feature
freeze, so I'm not inclined to be too patient, either.

...Robert

Attachment Content-Type Size
v12-0002-Rename-pg_verifybackup-s-JsonManifestParseContex.patch application/octet-stream 3.8 KB
v12-0001-Rename-JsonManifestParseContext-callbacks.patch application/octet-stream 5.3 KB
v12-0003-Move-src-bin-pg_verifybackup-parse_manifest.c-in.patch application/octet-stream 4.3 KB
v12-0004-Add-a-new-WAL-summarizer-process.patch application/octet-stream 135.0 KB
v12-0006-Add-new-pg_walsummary-tool.patch application/octet-stream 17.8 KB
v12-0005-Add-support-for-incremental-backup.patch application/octet-stream 212.1 KB
v12-0007-Test-patch-Enable-summarize_wal-by-default.patch application/octet-stream 4.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alena Rybakina 2023-11-30 14:50:36 Re: Implement missing join selectivity estimation for range types
Previous Message Ashutosh Bapat 2023-11-30 13:52:05 Re: Postgres Partitions Limitations (5.11.2.3)