GC mwindows on too many open files

[carlosmn]

We currently attempt to free unused map windows for packfiles when we go over a threshold. We however do not account for the number of open packfiles.

There's been at least two distinct reports of people running into issues when they have repositories which do network operations in small batches. These accumulate a lot of tiny packfiles and we end up having them all open at once, which can reach the system's limit for open file descriptors.

We should have a configurable limit on how many open file descriptors we want to have for packfiles and do a soft-GC similar to what we do for memory (address space).

[alexcrichton]

I believe we're hitting this in Cargo as well. We're using an index on GitHub which has lots of little files and lots of updates over time. Sometimes when we do a git update of the repository it causes a huge number of file descriptors to get opened, often reaching close to the system limit or blowing the system limit.

In tracking this down I found that this program is all that's needed to trigger the behavior, namely starting a revwalk that looks at the repository. I believe this is internally done during the fetch operation for smart transports.

Notably, though, I ran git gc in the repository and the problem goes away entirely. Both print statements in that program print 4, so no extra files are being created.

Out of curiosity, is there something we should be doing in Cargo to mitigate this issue? I'm gonna go run git gc manually in a few repos for now, but it'd be great if we could have Cargo do this for you!

[tommoor]

We have the same issue here and it's causing a lot of problems. However we are unable to depend on git being available locally (hence using libgit2) to perform gc. Are there other suggested workarounds?

[josharian]

I just this this as well (I believe), and I have the same issue as @tommoor; I can't necessarily rely on git being available locally to run git repack. And unfortunately I can't alter my ulimit. Are there other workarounds available?

[tommoor]

We had to abandon libgit2 unfortunately. We've rewritten in go-git – I'm sure that's not the answer you were hoping for 😞

[josharian]

@tommoor ouch. Thanks for the update. go-git has a repo.RepackObjects method, but I'm not quite ready to rewrite everything in go-git, and it seems silly to use two git libraries. :)

cc @ethomson @pks-t who seem to be the most active libgit2 folks at the moment, in case they have suggestions

[tommoor]

If you can do it from a licensing POV I'd recommend bundling the vanilla git binary with your application, it's much faster than both libgit2 and go-git and worth the extra weight if you ask me. Even github desktop bundles it.

[josharian]

It's worse than I thought. Even bundling vanilla git won't suffice, for a long-running process.

Watching lsof, the packfile fds never get closed, even if the underlying file gets deleted. This means means that as long as network operations keep happening, fds will continue to leak, regardless of whether the packfiles on disk get gc'd or not.

[josharian]

It looks like the fix is may be as simple as:

diff --git a/src/mwindow.c b/src/mwindow.c
index 262786a5f..ccb24bdd8 100644
--- a/src/mwindow.c
+++ b/src/mwindow.c
@@ -271,6 +271,9 @@ static git_mwindow *new_window(
        while (git_mwindow__mapped_limit < ctl->mapped &&
                        git_mwindow_close_lru(mwf) == 0) /* nop */;
 
+       while (ctl->open_windows > 20 &&
+               git_mwindow_close_lru(mwf) == 0) /* nop */;
+
        /*
         * We treat `mapped_limit` as a soft limit. If we can't find a
         * window to close and are above the limit, we still mmap the new

Obviously the limit needs to be made configurable. I'll see about putting together a PR. Feedback welcomed in the interim.

[josharian]

PR is #5386.