On Thu, Mar 31, 2016 at 2:19 PM, Magnus Hagander <mag...@hagander.net> wrote:
> On Thu, Mar 31, 2016 at 4:00 AM, David Steele <da...@pgmasters.net> wrote: > >> On 3/30/16 4:18 AM, Magnus Hagander wrote: >> > >> > On Wed, Mar 30, 2016 at 4:10 AM, David Steele <da...@pgmasters.net >> > <mailto:da...@pgmasters.net>> wrote: >> > >> > This certainly looks like it would work but it raises the barrier >> for >> > implementing backups by quite a lot. It's fine for backrest or >> barman >> > but it won't be pleasant for anyone who has home-grown scripts. >> > >> > >> > How much does it really raise the bar, though? >> > >> > It would go from "copy all files and make damn sure you copy pg_control >> > last, and rename it to pg_control.backup" to "take this binary blob you >> > got from the server and write it to pg_control.backup"? >> > >> > Also, the target of these APIs is specifically the backup tools and not >> > homewritten scripts. >> >> Then what would home-grown scripts use, the deprecated API that we know >> has issues? >> > > They should use either pg_basebackup/pg_receivexlog, or they should use a > tool like backrest or barman. > > > >> > A simple shellscript will have trouble enough using >> > it in the first place since it requires a persistent connection to the >> > database. >> >> All that's required is to spawn a psql process. I'm no shell expert but >> that's simple enough. >> > > Oh, it's certainly doable, but you also need to come back and talk to it > at a later time (to call stop backup), and do something useful with a > multiline output. None of that is particularly easy. Certainly doable, but > it becomes the wrong tool for the job. > > > >> > But those scripts are likely broken anyway. >> >> Yes, probably. Backup and especially archiving correctly are harder >> than most people realize. >> > > Exactly. Which is why we should discourage people from doing that, and > instead point them to the tools that do the job right. This is the whole > reason we're making this change in the first place. > > > >> > The main reason for Heikki to suggest this one over the other basic one >> > is that it brings protection against the "backup script/program crashed >> > halfway through but the user still tried to restore from that". They >> will >> > outright fail because there is no pg_control.backup in that case. >> >> But if we are going to make this complicated I'm not sure it's a big >> deal just to require pg_control to be copied last. pgBackRest already >> does that and Barman probably does, too. >> > > It does (I did doublecheck that at some point). > > > >> I don't see "don't copy pg_control" and "copy pg_control last" as all >> that different in terms of complexity. >> >> pgBackRest also *restores* pg_control last which I think is pretty >> important and would not be addressed by this solution. >> > > So maybe we should at least start that way. And just document that > clearly, and then use the patch that we have right now? > > Except we add so it still returns the stoppoint() as well (which is > returned from the current pg_stop_backup, but not in the new one). > Eh, nevermind. We do already return the stoppoint. I was looking at the wrong version of the patch. So basically that's current version of patch, but with proper docs of course. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
