Horton

Re: trying again to get incremental backup

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>, Peter Eisentraut <peter@eisentraut.org>, Dilip Kumar <dilipbalaut@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-11-16T17:13:53Z
Lists: pgsql-hackers 
On Thu, Nov 16, 2023 at 5:21 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> I meant code like this
>
>         memcpy(&key.rlocator, rlocator, sizeof(RelFileLocator));
>         key.forknum = forknum;
>         entry = blockreftable_lookup(brtab->hash, key);

Ah, I hadn't thought about that. Another way of handling that might be
to add = {0} to the declaration of key. But I can do the initializer
thing too if you think it's better. I'm not sure if there's an
argument that the initializer might optimize better.

> An output pointer, you mean :-)  (Should it be const?)

I'm bad at const, but that seems to work, so sure.

> When the return value is BACK_UP_FILE_FULLY, it's not clear what happens
> to these output values; we modify some, but why?  Maybe they should be
> left alone?  In that case, the "if size == 0" test should move a couple
> of lines up, in the brtentry == NULL block.

OK.

> BTW, you could do the qsort() after deciding to backup the file fully if
> more than 90% needs to be replaced.

OK.

> BTW, in sendDir() why do
>  lookup_path = pstrdup(pathbuf + basepathlen + 1);
> when you could do
>  lookup_path = pstrdup(tarfilename);
> ?

No reason, changed.

> > If we want to inject more underscores here, my vote is to go all the
> > way and make it per_wal_range_cb.
>
> +1

Will look into this.

> Yeah, I just think that endless stream of hex chars are hard to read,
> and I've found myself following digits in the screen with my fingers in
> order to parse file names.  I guess you could say thousands separators
> for regular numbers aren't needed either, but we do have them for
> readability sake.

Sigh.

> I think a new section in chapter 30 "Reliability and the Write-Ahead
> Log" is warranted.  It would explain the summarization process, what the
> summary files are used for, and the deletion mechanism.  I can draft
> something if you want.

Sure, if you want to take a crack at it, that's great.

> It's not clear to me if WalSummarizerCtl->pending_lsn if fulfilling some
> purpose or it's just a leftover from prior development.  I see it's only
> read in an assertion ... Maybe if we think this cross-check is
> important, it should be turned into an elog?  Otherwise, I'd remove it.

I've been thinking about that. One thing I'm not quite sure about
though is introspection. Maybe there should be a function that shows
summarized_tli and summarized_lsn from WalSummarizerData, and maybe it
should expose pending_lsn too.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Commits

  1. Minor fixes to pg_combinebackup and its documentation.

  2. Fix defects in PrepareForIncrementalBackup.

  3. Add WALSummarizerLock to wait_event_names.txt

  4. Initialize variable to placate compiler.

  5. Replace nonsense comment with a relevant one.

  6. Fix numerous typos in incremental backup commits.

  7. Add support for incremental backup.

  8. Add a new WAL summarizer process.

  9. Move src/bin/pg_verifybackup/parse_manifest.c into src/common.

  10. Fix brown paper bag bug in 5c47c6546c413d5eb51c1626070a807026e6139d.

  11. Rename pg_verifybackup's JsonManifestParseContext callback functions.

  12. Rename JsonManifestParseContext callbacks.

  13. Change how a base backup decides which files have checksums.

  14. Change struct tablespaceinfo's oid member from 'char *' to 'Oid'

  15. Refactor parse_filename_for_nontemp_relation to parse more.

  16. During online checkpoints, insert XLOG_CHECKPOINT_REDO at redo point.

  17. In basebackup.c, refactor to create read_file_data_into_buffer.

  18. In basebackup.c, refactor to create verify_page_checksum.

  19. Report syncscan position at end of scan.

  20. Exclude additional directories in pg_basebackup

  21. Add new JSON processing functions and parser API.