Re: pg_basebackup stream xlog to tar

On Sat, Oct 15, 2016 at 8:51 AM, Magnus Hagander <magnus@hagander.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