Re: pg_basebackup stream xlog to tar

On Sat, Oct 15, 2016 at 8:51 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Fixed.

Ok, I had a extra look on the patch:
+ <para>The transactionn log files are written to a separate file
+ called <filename>pg_xlog.tar</filename>.
+ </para>
s/transactionn/transaction/, and the <para> markup should be on its own line.

+ if (dir_data->sync)
+ {
+ if (fsync_fname(tmppath, false, progname) != 0)
+ {
+ close(fd);
+ return NULL;
+ }
+ if (fsync_parent_path(tmppath, progname) != 0)
+ {
+ close(fd);
+ return NULL;
+ }
+ }
Nit: squashing both things together would simplify the code.

+ else if (method == CLOSE_UNLINK
+ )
Your finger slipped here.

Except that it looks in pretty good to me, so I am switching that as
ready for committer.

> But independent of this patch, actually putting that test in for non-tar
> mode would probably not be a bad idea -- if that breaks, it's likely both
> break, after all.

Agreed (you were able to break only tar upthread with your patch). One
way to do that elegantly would be to:
1) extend slurp_dir to return only files that have a matching pattern.
That's not difficult to do:
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -184,10 +184,14 @@ sub generate_ascii_string

sub slurp_dir
{
- my ($dir) = @_;
+ my ($dir, $match_pattern) = @_;
opendir(my $dh, $dir)
or die "could not opendir \"$dir\": $!";
my @direntries = readdir $dh;
+ if (defined($match_pattern))
+ {
+ @direntries = grep($match_pattern, @direntries);
+ }
closedir $dh;
return @direntries;
}
Sorting them at the same time may be a good idea..
2) Add an option to pg_xlogdump to be able to output its output to a
file. That would be awkward to rely on grabbing the output data from a
pipe... On Windows particularly. Thinking about it, would that
actually be useful to others? That's not a complicated patch.
--
Michael