On Wed, Jan 19, 2011 at 06:14, Fujii Masao <masao.fujii@gmail.com> wrote:
> 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"
Oops.
> + 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"?
Hmm. "fast or spread" seems reasonable to me. And I want to use "fast"
for the fast version, because that's what we call it when you use
pg_start_backup(). I'll go change it to spread for now - it's the one
I can find used in the docs.
> You seem to have forgotten to support "--checkpoint" long option.
> The struct long_options needs to be updated.
Wow, that clearly went too fast. Fixed as wlel.
> 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?
Nah, we'll just disconnect and it'll deal with things that way. Just
like we do with e.g. pg_dump. I don't see the need to complicate it
with that.
(new patch on github in 5 minutes)
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
