Hi, I have an unexpected 5 mins window to do a first reading of the patch, so here goes the quick doc and comments proof reading of it. :)
Magnus Hagander <mag...@hagander.net> writes: > This patch creates pg_basebackup in bin/, being a client program for > the streaming base backup feature. Great! We have pg_ctl init[db], I think we want pg_ctl clone or some other command here to call the binary for us. What do you think? > One thing I'm thinking about - right now the tool just takes -c > <conninfo> to connect to the database. Should it instead be taught to > take the connection parameters that for example pg_dump does - one for > each of host, port, user, password? (shouldn't be hard to do..) Consistency is good. Now, basic first patch reading level review: I think doc/src/sgml/backup.sgml should include some notes about how libpq base backup streaming compares to rsync and the like in term of efficiency or typical performances, when to prefer which, etc. I'll see about doing some tests next week. + <term><option>--basedir=<replaceable class="parameter">directory</replaceable></option></term> That should be -D --pgdata, for consistency with pg_dump. On a quick reading it's unclear from the docs alone how -d and -t leave together. It seems like the options are exclusive but I'd have to ask… + * The file will be named base.tar[.gz] if it's for the main data directory + * or <tablespaceoid>.tar[.gz] if it's for another tablespace. Well we have UNIQUE, btree (spcname), so maybe we can use that here? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
