Re: trying again to get incremental backup
Robert Haas <robertmhaas@gmail.com>
Attachments
- v3-0002-Refactor-parse_filename_for_nontemp_relation-to-p.patch (application/octet-stream) patch v3-0002
- v3-0004-Move-src-bin-pg_verifybackup-parse_manifest.c-int.patch (application/octet-stream) patch v3-0004
- v3-0001-Change-struct-tablespaceinfo-s-oid-member-from-ch.patch (application/octet-stream) patch v3-0001
- v3-0003-Change-how-a-base-backup-decides-which-files-have.patch (application/octet-stream) patch v3-0003
- v3-0005-Prototype-patch-for-incremental-and-differential-.patch (application/octet-stream) patch v3-0005
- v3-0006-Add-new-pg_walsummary-tool.patch (application/octet-stream) patch v3-0006
- v3-0007-Add-TAP-tests-this-is-broken-doesn-t-work.patch (application/octet-stream) patch v3-0007
On Tue, Sep 12, 2023 at 5:56 AM Dilip Kumar <dilipbalaut@gmail.com> wrote: > + BlockNumber relative_block_numbers[RELSEG_SIZE]; > > This is close to 400kB of memory, so I think it is better we palloc it > instead of keeping it in the stack. Fixed. > Unrelated code refactoring hunk Fixed. > This structure is not used anywhere. Removed. > /If the file is to be set incrementally/If the file is to be sent incrementally Fixed. > I do not really like this change, because after removing this you have > put 2 independent checks for sending the full file[1] and sending it > incrementally[1]. Actually for sending incrementally > 'statbuf->st_size' is computed from the 'num_incremental_blocks' > itself so why don't we keep this breaking condition in the while loop > itself? So that we can avoid these two separate conditions. I don't think that would be correct. The number of bytes that need to be read from the original file is not equal to the number of bytes that will be written to the incremental file. Admittedly, they're currently different by less than a block, but that could change if we change the format of the incremental file (e.g. suppose we compressed the blocks in the incremental file with gzip, or smushed out the holes in the pages). I wrote the loop as I did precisely so that the two cases could have different loop exit conditions. > Better we add some comments for these structures. Done. Here's a new patch set, also addressing Jakub's observation that MINIMUM_VERSION_FOR_WAL_SUMMARIES needed updating. -- Robert Haas EDB: http://www.enterprisedb.com
Commits
-
Minor fixes to pg_combinebackup and its documentation.
- 1713e3d6cd39 17.0 cited
-
Fix defects in PrepareForIncrementalBackup.
- dffde5bf16a5 17.0 landed
-
Add WALSummarizerLock to wait_event_names.txt
- 5c430f9dc559 17.0 landed
-
Initialize variable to placate compiler.
- da083b20f637 17.0 landed
-
Replace nonsense comment with a relevant one.
- ffc6ab9b56ae 17.0 landed
-
Fix numerous typos in incremental backup commits.
- 49f2194ed5c1 17.0 landed
-
Add support for incremental backup.
- dc212340058b 17.0 landed
-
Add a new WAL summarizer process.
- 174c480508ac 17.0 landed
-
Move src/bin/pg_verifybackup/parse_manifest.c into src/common.
- aafc07c7a191 17.0 landed
-
Fix brown paper bag bug in 5c47c6546c413d5eb51c1626070a807026e6139d.
- 47f01d727e3a 17.0 landed
-
Rename pg_verifybackup's JsonManifestParseContext callback functions.
- 278eb13c4823 17.0 landed
-
Rename JsonManifestParseContext callbacks.
- d463aa06a9a8 17.0 landed
-
Change how a base backup decides which files have checksums.
- 025584a168a4 17.0 landed
-
Change struct tablespaceinfo's oid member from 'char *' to 'Oid'
- 5b36e8f078a3 17.0 landed
-
Refactor parse_filename_for_nontemp_relation to parse more.
- 5c47c6546c41 17.0 landed
-
During online checkpoints, insert XLOG_CHECKPOINT_REDO at redo point.
- afd12774ae89 17.0 landed
-
In basebackup.c, refactor to create read_file_data_into_buffer.
- c2ba3fdea593 17.0 landed
-
In basebackup.c, refactor to create verify_page_checksum.
- 053183138a7a 17.0 landed
-
Report syncscan position at end of scan.
- e8d74ad625f7 17.0 cited
-
Exclude additional directories in pg_basebackup
- 6ad8ac602628 10.0 cited
-
Add new JSON processing functions and parser API.
- a570c98d7fa0 9.3.0 cited