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

From: Björn Steinbrink <b.steinbrink@gmx.de>
Date: Jun 29, 2008, 00:58 UTC
Message-ID: <20080629005858.GA2036@atjola.homenet>
In-Reply-To: <1214696036-8294-1-git-send-email-apenwarr@gmail.com>

On 2008.06.28 19:33:56 -0400, Avery Pennarun wrote:

Show 20 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.

Oh sweet! That also means that I don't have to fear about the repos I already track with git-svn being corrupted (at least I hope so). :-)

And yeah, looking at /tmp, there are still 58K temp files belonging to the failed import. Temporarily mounting a 8k tmpfs there triggers the bug immediately, while with this patch I get a error message.

I'm running another import of the coq repo that failed earlier, just to make sure, but I also just noticed that cat-file --batch is leaking memory. For example this one shoots up to about 700M RSS usage with git.git:

git rev-list --objects origin/master | \
	sed 's/ .*//' | \
	git cat-file --batch > /dev/null

I'll follow-up with a patch that at least fixes the worst part of that, getting the RSS usage for the above test down to about 40M.

Thanks! Björn