On Tue, Nov 1, 2011 at 3:08 AM, Magnus Hagander <mag...@hagander.net> wrote: > On Fri, Oct 28, 2011 at 08:46, Fujii Masao <masao.fu...@gmail.com> wrote: >> On Thu, Oct 27, 2011 at 11:14 PM, Magnus Hagander <mag...@hagander.net> >> wrote: >>> Here's a version that does this. Turns out this requires a lot less >>> code than what was previously in there, which is always nice. >>> >>> We still need to solve the other part which is how to deal with the >>> partial files on restore. But this is definitely a cleaner way from a >>> pure pg_receivexlog perspective. >>> >>> Comments/reviews? >> >> Looks good. >> >> Minor comment: >> the source code comment of FindStreamingStart() seems to need to be updated. > > Here's an updated patch that both includes this update to the comment, > and also the functionality to pre-pad files to 16Mb. This also seems > to have simplified the code, which is a nice bonus.
Here are the comments: In open_walfile(), "zerobuf" needs to be free'd after use of it. + f = open(fn, O_WRONLY | O_CREAT | PG_BINARY, 0666); We should use "S_IRUSR | S_IWUSR" instead of "0666" as a file access modes? + if (write(f, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ) + { + fprintf(stderr, _("%s: could not pad WAL segment %s: %s\n"), + progname, fn, strerror(errno)); + close(f); + return -1; + } When write() fails, we should delete the partial WAL file, like XLogFileInit() does? If not, subsequent pg_receivexlog always fails unless a user deletes it manually. Because open_walfile() always fails when it finds an existing partial WAL file. When open_walfile() fails, pg_receivexlog exits without closing the connection. I don't think this is good error handling. But this issue itself is not what we're trying to address now. So I think you can commit separately from current patch. 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