gitserver: Made git GC recloning less dependent on time#62776
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
0f5c133 to
7d9ada6
Compare
There was a problem hiding this comment.
fs.ReadFile on a file is much cheaper than spawning a whole process.
What is our logic then for when it warrants putting stuff in git config and when not to?
There was a problem hiding this comment.
Given the issues we've seen from storing sg specific things in git config, I think we should only store config in there that git itself cares about - everything else we can do with lower IOPS / process spawning overhead ourselves.
There was a problem hiding this comment.
Note that we could also use go-git to modify the git config in a structured way (if we want to keep the property of storing things in the git config without spawning the process). I don't feel strongly about this though.
There was a problem hiding this comment.
I thought about switching (had a very old branch actually that switched those to go-git), but when I looked at the implementation it didn't seem to use the same lockfile mechanism that git itself uses so it would probably not be thread safe and come with its own challenges :(
There was a problem hiding this comment.
Random number to get started - 1 seems too risky, 5 is not 10 😆 I want to tune this as we learn more about the likelihood of this happening
7d9ada6 to
f7165cb
Compare
On small instances, we gave git GC a lot of time to recover after a failure. However, on large instances, the time based approach could mean that after only a single failed attempt we will already reclone a repo. This PR changes that to use a counter instead, and moves away from git config for that, fs.ReadFile on a file is much cheaper than spawning a whole process. Test plan: Tests still pass, added some new tests.
On small instances, we gave git GC a lot of time to recover after a failure. However, on large instances, the time based approach could mean that after only a single failed attempt we will already reclone a repo.
This PR changes that to use a counter instead, and moves away from git config for that, fs.ReadFile on a file is much cheaper than spawning a whole process.
Closes https://github.com/sourcegraph/sourcegraph/issues/62649
Test plan:
Tests still pass, added some new tests.