Re: pg_basebackup for streaming base backups - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: pg_basebackup for streaming base backups
Date
Msg-id AANLkTinBZNa5DjBSxcTNCPX9K+ZBqY=Y8_6rgFqQzN6u@mail.gmail.com
Whole thread Raw
In response to Re: pg_basebackup for streaming base backups  (Magnus Hagander <magnus@hagander.net>)
Responses Re: pg_basebackup for streaming base backups
List pgsql-hackers
On Wed, Jan 19, 2011 at 4:12 AM, Magnus Hagander <magnus@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


pgsql-hackers by date:

Previous
From: Joachim Wieland
Date:
Subject: Re: Snapshot synchronization, again...
Next
From: Stephen Frost
Date:
Subject: Re: Add support for logging the current role