Re: [PATCH v2] git-svn: avoid filling up the disk with temp files.

[Eric Wong <normalperson@yhbt.net>]

Avery Pennarun <apenwarr@gmail.com> wrote:
> 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.

>  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