On Thu, Apr 16, 2015 at 01:35:05PM +0200, Andreas Mohr wrote:
> I strongly suspect that git's repacking implementation
> (probably unrelated to msysgit-specific deviations,
> IOW, git *core* handling)
> simply is buggy
> in that it may keep certain file descriptors open
> at least a certain time (depending on scope of implementation/objects!?)
> beyond having finished its operation (rename?).
Hrm. I do not see anything in builtin/fetch.c that closes the packfile
descriptors before running "gc --auto". So basically the sequence:
1. Fetch performs actual fetch. It needs to open packfiles to do
commit negotiation with other side (the hard work is done
by an index-pack subprocess, but it is likely we have to access
_some_ objects).
2. The packfiles remain open and mmap'd (at least on Linux) in the
sha1_file.c:packed_git list.
3. We spawn "gc --auto" and wait for it to finish. While we are
waiting, the descriptors are still open, but "gc --auto" will not be
able to delete any packs.
But this seems too simple to be the problem, as it would mean that just
about any "gc --auto" that triggers a full repack would be a problem (so
anytime you have about 50 packs). But maybe the gc "autodetach" behavior
means it works racily.
I was able to set up the situation deterministically by running the
script below:
-- >8 --
#!/bin/sh
# XXX tweak this setting as appropriate
PATH_TO_GIT_BUILD=$HOME/compile/git
PATH=$PATH_TO_GIT_BUILD/bin-wrappers:$PATH
rm -rf parent child
# make a parent/child where the child will have to access
# a packfile to fulfill another fetch
git init parent &&
git -C parent commit --allow-empty -m base &&
git clone parent child &&
git -C parent commit --allow-empty -m extra &&
# we want to make our base pack really big, because otherwise
# git will open/mmap/close it. So we must exceed core.packedgitlimit
cd child &&
$PATH_TO_GIT_BUILD/test-genrandom foo 5000000 >file &&
git add file &&
git commit -m large file &&
git repack -ad &&
git config core.packedGitLimit 1M &&
# now make some spare packs to bust the gc.autopacklimit
for i in 1 2 3 4 5; do
git commit --allow-empty -m $i &&
git repack -d
done &&
git config gc.autoPackLimit 3 &&
git config gc.autoDetach false &&
GIT_TRACE=1 git fetch
```
I also instrumented my (v1.9.5) git build like this:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 025bc3e..fc99e5e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1174,6 +1174,12 @@ int cmd_fetch(int argc, const char **argv, const char
*prefix)
list.strdup_strings = 1;
string_list_clear(&list, 0);
+ {
+ struct packed_git *p;
+ for (p = packed_git; p; p = p->next)
+ trace_printf("pack %s has descriptor %d\n",
+ p->pack_name, p->pack_fd);
+ }
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
return result;
diff --git a/builtin/repack.c b/builtin/repack.c
index bb2314c..e8b29cf 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -105,6 +105,7 @@ static void remove_redundant_pack(const char *dir_name,
const char *base_name)
for (i = 0; i < ARRAY_SIZE(exts); i++) {
strbuf_setlen(&buf, plen);
strbuf_addstr(&buf, exts[i]);
+ trace_printf("unlinking %s\n", buf.buf);
unlink(buf.buf);
}
strbuf_release(&buf);
to confirm what was happening (because of course on Linux it is
perfectly fine to delete the open file). If this does trigger the bug
for you, though, it should be obvious even without the trace calls. :)
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
