Re: trying again to get incremental backup
Robert Haas <robertmhaas@gmail.com>
On Fri, Oct 20, 2023 at 11:30 AM David Steele <david@pgmasters.net> wrote: > Then I'm fine with it as is. OK, thanks. > In my view this feature puts the cart way before the horse. I'd think > higher priority features might be parallelism, a backup repository, > expiration management, archiving, or maybe even a restore command. > > It seems the only goal here is to make pg_basebackup a tool for external > backup software to use, which might be OK, but I don't believe this > feature really advances pg_basebackup as a usable piece of stand-alone > software. If people really think that start/stop backup is too > complicated an interface how are they supposed to track page > incrementals and get them to a place where pg_combinebackup can put them > backup together? If automation is required to use this feature, > shouldn't pg_basebackup implement that automation? > > I have plenty of thoughts about the implementation as well, but I have a > lot on my plate right now and I don't have time to get into it. > > I don't plan to stand in your way on this feature. I'm reviewing what > patches I can out of courtesy and to be sure that nothing adjacent to > your work is being affected. My apologies if my reviews are not meeting > your expectations, but I am contributing as my time constraints allow. Sorry, I realize reading this response that I probably didn't do a very good job writing that email and came across sounding like a jerk. Possibly, I actually am a jerk. Whether it just sounded like it or I actually am, I apologize. But your last paragraph here gets at my real question, which is whether you were going to try to block the feature. I recognize that we have different priorities when it comes to what would make most sense to implement in PostgreSQL, and that's OK, or at least, it's OK with me. I also don't have any particular expectation about how much you should review the patch or in what level of detail, and I have sincerely appreciated your feedback thus far. If you are able to continue to provide more, that's great, and if that's not, well, you're not obligated. What I was concerned about was whether that review was a precursor to a vigorous attempt to keep the main patch from getting committed, because if that was going to be the case, then I'd like to surface that conflict sooner rather than later. It sounds like that's not an issue, which is great. At the risk of drifting into the fraught question of what I *should* be implementing rather than the hopefully-less-fraught question of whether what I am actually implementing is any good, I see incremental backup as a way of removing some of the use cases for the low-level backup API. If you said "but people still will have lots of reasons to use it," I would agree; and if you said "people can still screw things up with pg_basebackup," I would also agree. Nonetheless, most of the disasters I've personally seen have stemmed from the use of the low-level API rather than from the use of pg_basebackup, though there are exceptions. I also think a lot of the use of the low-level API is driven by it being just too darn slow to copy the whole database, and incremental backup can help with that in some circumstances. Also, I have worked fairly hard to try to make sure that if you misuse pg_combinebackup, or fail to use it altogether, you'll get an error rather than silent data corruption. I would be interested to hear about scenarios where the checks that I've implemented can be defeated by something that is plausibly described as stupidity rather than malice. I'm not sure we can fix all such cases, but I'm very alert to the horror that will befall me if user error looks exactly like a bug in the code. For my own sanity, we have to be able to distinguish those cases. Moreover, we also need to be able to distinguish backup-time bugs from reassembly-time bugs, which is why I've got the pg_walsummary tool, and why pg_combinebackup has the ability to emit fairly detailed debugging output. I anticipate those things being useful in investigating bug reports when they show up. I won't be too surprised if it turns out that more work on sanity-checking and/or debugging tools is needed, but I think your concern about people misusing stuff is bang on target and I really want to do whatever we can to avoid that when possible and detect it when it happens. -- 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