Re: Improve shutdown during online backup, take 3

Albe Laurenz wrote:
> Simon Riggs wrote:
> > Patch applies, and works as described. Looks good for final apply.
> > 
> > Few minor thoughts:
> > 
> > * Text in pg_ctl should be WARNING, not Warning.
> > * CancelBackup() API looks strange, not sure why
> > * Need to mention that CancelBackup() is not the right way to end a
> > backup, so that function and pg_stop_backup should reference 
> > each other
> > 
> > Other than those, I like it. Very useful patch.
> 
> Thanks for the feedback!
> 
> - I have replaced "Warning" with WARNING".
> - I have changed the API of CancelBackup() to return void.
>   I don't use the return code anyway, and I guess it's less confusing
>   and "strange" that way.
> - I have added comments to disambiguate pg_stop_backup() and
> CancelBackup().
> 
> CancelBackup now writes a message to the server log if it cannot
> delete backup_label - I hope that's not too verbose...

This doesn't look like our normal coding standards, and should probably
be changed:
+ 	if (0 != stat(BACKUP_LABEL_FILE, &stat_buf))

(there's a number of similar places)

//Magnus

-- 
Sent via pgsql-patches mailing list (pgsql-patches@[EMAIL PROTECTED]
)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches