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.
I incorporated those changes before pushing.
> 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.
I think both of those would be worthwhile. Just for the testability in itself, but such a flag to pg_xlogdump would probably be useful in other cases as well, beyond just the testing.
