On Wed, Jan 19, 2011 at 4:12 AM, Magnus Hagander <mag...@hagander.net> wrote:
> Ah, ok. I've added the errorcode now, PFA. I also fixed an error in
> the change for result codes I broke in the last patch. github branch
> updated as usual.

Great. Thanks for the quick update!

Here are another comments:

+ * IDENTIFICATION
+ *               src/bin/pg_basebackup.c

Typo: s/"src/bin/pg_basebackup.c"/"src/bin/pg_basebackup/pg_basebackup.c"


+       printf(_("  -c, --checkpoint=fast|slow\n"
+                        "                            set fast or slow 
checkpoinging\n"));

Typo: s/checkpoinging/checkpointing

The "fast or slow" seems to lead users to always choose "fast". Instead,
what about "fast or smooth", "fast or spread" or "immediate or delayed"?

You seem to have forgotten to support "--checkpoint" long option.
The struct long_options needs to be updated.

What if pg_basebackup receives a signal while doing a backup?
For example, users might do Ctrl-C to cancel the long-running backup.
We should define a signal handler and send a Terminate message
to the server to cancel the backup?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to