From e183530550dc1b73d24fb5ae7d84e85286e88ffb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 11 Sep 2017 22:02:58 -0400 Subject: [PATCH] Fix RecursiveCopy.pm to cope with disappearing files. When copying from an active database tree, it's possible for files to be deleted after we see them in a readdir() scan but before we can open them. (Once we've got a file open, we don't expect any further errors from it getting unlinked, though.) Tweak RecursiveCopy so it can cope with this case, so as to avoid irreproducible test failures. Back-patch to 9.6 where this code was added. In v10 and HEAD, also remove unused "use RecursiveCopy" in one recovery test script. Michael Paquier and Tom Lane Discussion: https://postgr.es/m/24621.1504924323@sss.pgh.pa.us --- src/test/perl/RecursiveCopy.pm | 66 +++++++++++++------ .../t/010_logical_decoding_timelines.pl | 1 - 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/test/perl/RecursiveCopy.pm b/src/test/perl/RecursiveCopy.pm index 28ecaf6db2..19f7dd2fff 100644 --- a/src/test/perl/RecursiveCopy.pm +++ b/src/test/perl/RecursiveCopy.pm @@ -29,12 +29,17 @@ use File::Copy; =head2 copypath($from, $to, %params) Recursively copy all files and directories from $from to $to. +Does not preserve file metadata (e.g., permissions). Only regular files and subdirectories are copied. Trying to copy other types of directory entries raises an exception. Raises an exception if a file would be overwritten, the source directory can't -be read, or any I/O operation fails. Always returns true. +be read, or any I/O operation fails. However, we silently ignore ENOENT on +open, because when copying from a live database it's possible for a file/dir +to be deleted after we see its directory entry but before we can open it. + +Always returns true. If the B parameter is given, it must be a subroutine reference. This subroutine will be called for each entry in the source directory with its @@ -74,6 +79,9 @@ sub copypath $filterfn = sub { return 1; }; } + # Complain if original path is bogus, because _copypath_recurse won't. + die "\"$base_src_dir\" does not exist" if !-e $base_src_dir; + # Start recursive copy from current directory return _copypath_recurse($base_src_dir, $base_dest_dir, "", $filterfn); } @@ -89,12 +97,8 @@ sub _copypath_recurse return 1 unless &$filterfn($curr_path); # Check for symlink -- needed only on source dir - die "Cannot operate on symlinks" if -l $srcpath; - - # Can't handle symlinks or other weird things - die "Source path \"$srcpath\" is not a regular file or directory" - unless -f $srcpath - or -d $srcpath; + # (note: this will fall through quietly if file is already gone) + die "Cannot operate on symlink \"$srcpath\"" if -l $srcpath; # Abort if destination path already exists. Should we allow directories # to exist already? @@ -104,25 +108,47 @@ sub _copypath_recurse # same name and we're done. if (-f $srcpath) { - copy($srcpath, $destpath) + my $fh; + unless (open($fh, '<', $srcpath)) + { + return 1 if ($!{ENOENT}); + die "open($srcpath) failed: $!"; + } + copy($fh, $destpath) or die "copy $srcpath -> $destpath failed: $!"; + close $fh; return 1; } - # Otherwise this is directory: create it on dest and recurse onto it. - mkdir($destpath) or die "mkdir($destpath) failed: $!"; - - opendir(my $directory, $srcpath) or die "could not opendir($srcpath): $!"; - while (my $entry = readdir($directory)) + # If it's a directory, create it on dest and recurse into it. + if (-d $srcpath) { - next if ($entry eq '.' or $entry eq '..'); - _copypath_recurse($base_src_dir, $base_dest_dir, - $curr_path eq '' ? $entry : "$curr_path/$entry", $filterfn) - or die "copypath $srcpath/$entry -> $destpath/$entry failed"; - } - closedir($directory); + my $directory; + unless (opendir($directory, $srcpath)) + { + return 1 if ($!{ENOENT}); + die "opendir($srcpath) failed: $!"; + } - return 1; + mkdir($destpath) or die "mkdir($destpath) failed: $!"; + + while (my $entry = readdir($directory)) + { + next if ($entry eq '.' or $entry eq '..'); + _copypath_recurse($base_src_dir, $base_dest_dir, + $curr_path eq '' ? $entry : "$curr_path/$entry", $filterfn) + or die "copypath $srcpath/$entry -> $destpath/$entry failed"; + } + + closedir($directory); + return 1; + } + + # If it disappeared from sight, that's OK. + return 1 if !-e $srcpath; + + # Else it's some weird file type; complain. + die "Source path \"$srcpath\" is not a regular file or directory"; } 1; diff --git a/src/test/recovery/t/010_logical_decoding_timelines.pl b/src/test/recovery/t/010_logical_decoding_timelines.pl index edc0219c9c..5620450acf 100644 --- a/src/test/recovery/t/010_logical_decoding_timelines.pl +++ b/src/test/recovery/t/010_logical_decoding_timelines.pl @@ -24,7 +24,6 @@ use warnings; use PostgresNode; use TestLib; use Test::More tests => 13; -use RecursiveCopy; use File::Copy; use IPC::Run (); use Scalar::Util qw(blessed);