Re: [PATCH v2] git-svn: avoid filling up the disk with temp files.
Avery Pennarun <apenwarr@gmail.com> wrote:
Show 23 quoted lines
> Commit ffe256f9bac8a40ff751a9341a5869d98f72c285 ("git-svn: Speed up fetch")
> introduced changes that create a temporary file for each object fetched by
> svn. These files should be deleted automatically, but perl apparently
> doesn't do this until the process exits (or perhaps when its garbage
> collector runs).
>
> This means that on a large fetch, especially with lots of branches, we
> sometimes fill up /tmp completely, which prevents the next temp file from
> being written completely. This is aggravated by the fact that a new temp
> file is created for each updated file, even if that update produces a file
> identical to one already in git. Thus, it can happen even if there's lots
> of disk space to store the finished repository.
>
> We weren't adequately checking for write errors, so this would result in an
> invalid file getting committed, which caused git-svn to fail later with an
> invalid checksum.
>
> This patch adds a check to syswrite() so similar problems don't lead to
> corruption in the future. It also unlink()'s each temp file explicitly
> when we're done with it, so the disk doesn't need to fill up.
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
> ---> Please use this in favour of the "Revert "git-svn: Speed up fetch" I sent
> earlier. I ended up having a surprise inspiration that led to a real fix :)
Ouch, I didn't noticed these unchecked syscalls :x
Very graciously
Acked-by: Eric Wong <normalperson@yhbt.net>
Apologies to all users who were bitten by this bug.
Show 28 quoted lines
> git-svn.perl | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 263d66c..0011387 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -3243,7 +3243,9 @@ sub close_file {
> my ($tmp_fh, $tmp_filename) = File::Temp::tempfile(UNLINK => 1);
> my $result;
> while ($result = sysread($fh, my $string, 1024)) {
> - syswrite($tmp_fh, $string, $result);
> + my $wrote = syswrite($tmp_fh, $string, $result);
> + defined($wrote) && $wrote == $result
> + or croak("write $tmp_filename: $!\n");
> }
> defined $result or croak $!;
> close $tmp_fh or croak $!;
> @@ -3251,6 +3253,7 @@ sub close_file {
> close $fh or croak $!;
>
> $hash = $::_repository->hash_and_insert_object($tmp_filename);
> + unlink($tmp_filename);
> $hash =~ /^[a-f\d]{40}$/ or die "not a sha1: $hash\n";
> close $fb->{base} or croak $!;
> } else {
> --
> 1.5.4.3