Re: trying again to get incremental backup
От | David Steele |
---|---|
Тема | Re: trying again to get incremental backup |
Дата | |
Msg-id | 590e3017-da1f-4af6-9bf0-1679511ca7e5@pgmasters.net обсуждение исходный текст |
Ответ на | Re: trying again to get incremental backup (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: trying again to get incremental backup
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
On 10/19/23 16:00, Robert Haas wrote: > On Thu, Oct 19, 2023 at 3:18 PM David Steele <david@pgmasters.net> wrote: >> 0001 looks pretty good to me. The only thing I find a little troublesome >> is the repeated construction of file names with/without segment numbers >> in ResetUnloggedRelationsInDbspaceDir(), .e.g.: >> >> + if (segno == 0) >> + snprintf(dstpath, sizeof(dstpath), "%s/%u", >> + dbspacedirname, relNumber); >> + else >> + snprintf(dstpath, sizeof(dstpath), "%s/%u.%u", >> + dbspacedirname, relNumber, segno); >> >> >> If this happened three times I'd definitely want a helper function, but >> even with two I think it would be a bit nicer. > > Personally I think that would make the code harder to read rather than > easier. I agree that repeating code isn't great, but this is a > relatively brief idiom and pretty self-explanatory. If other people > agree with you I can change it, but to me it's not an improvement. Then I'm fine with it as is. >> 0002 is definitely a good idea. FWIW pgBackRest does this conversion but >> also errors if it does not succeed. We have never seen a report of this >> error happening in the wild, so I think it must be pretty rare if it >> does happen. > > Cool, but ... how about the main patch set? It's nice to get some of > these refactoring bits and pieces out of the way, but if I spend the > effort to work out what I think are the right answers to the remaining > design questions for the main patch set and then find out after I've > done all that that you have massive objections, I'm going to be > annoyed. I've been trying to get this feature into PostgreSQL for > years, and if I don't succeed this time, I want the reason to be > something better than "well, I didn't find out that David disliked X > until five minutes before I was planning to type 'git push'." I simply have not had time to look at the main patch set in any detail. > I'm not really concerned about detailed bug-hunting in the main > patches just yet. The time for that will come. But if you have views > on how to resolve the design questions that I mentioned in a couple of > emails back, or intend to advocate vigorously against the whole > concept for some reason, let's try to sort that out sooner rather than > later. 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. Regards, -David
В списке pgsql-hackers по дате отправления:
Предыдущее
От: "Zhijie Hou (Fujitsu)"Дата:
Сообщение: RE: [PoC] pg_upgrade: allow to upgrade publisher node
Следующее
От: "Tristan Partin"Дата:
Сообщение: Re: controlling meson's parallelism (and some whining)