On Sun, Feb 20, 2011 at 21:37, Dimitri Fontaine <dimi...@2ndquadrant.fr> wrote: > Hi, > > Magnus Hagander <mag...@hagander.net> writes: >> Better late than never (or?), here's the final cleanup of >> pg_streamrecv for moving into the main distribution, per discussion >> back in late dec or early jan. It also includes the "stream logs in >> parallel to backup" part that was not completed on pg_basebackup. > > And that's something I've been so interested in! It's only fair game > that I spend time reviewing after my insisting for having it :) > > The programs (pg_basebackup and pg_receivexlog) both work as expected, > and show in the pg_stat_replication system view. > > pg_basebackup -x option should be revised so that it's easier to make > the difference between streaming WAL while the base backup is ongoing > and fetching them at the end, taking the risk to corrupt the whole > backup as soon as wal_keep_segments is undersized. > > -x, --xlog[=stream] include required WAL files in backup > > It could be --xlog=stream|fetch or something that reads better.
Yeha, that's probably true. I wanted to avoid making it mandatory, but it's actually easier this way. Will change it to that. > Now, on to the code itself. > > I wonder if the segment_callback() routine would better be a while{} > loop rather than a recursive construct. Also, it looks like a lib > function but it's doing exit(1)… Actually, it's even better to just reorder the checks in the other order - that way we don't need a loop *or* a self-call. > Unfortunately I can't comment (or won't risk learning enough details > tonight to try to be smart here) on FindStreamingStart() implementation, > that seems crucial. It is - so if you can find the time to, that would be great... > Will the server refrain from recycling a WAL file when all receivers > sent_location are not known to be past the positions contained in it? > If that's the case, the documentation should talk about pg_receivexlog > as an alternative to archiving, relying on libpq. It that's not the > case, is there a good reason for that not being the case? (even if > that's not on this patch to fix that). No, not at this point. It would be nice to have that option in the future... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers