optimize pullRemote for large repos

[coopernetes]

Is your feature request related to a problem? Please describe.
On particularly large repositories as well as when GitProxy is deployed in a production-like environment (constrained or at least defined upper limits on CPU/memory), pullRemote step will sometimes cause the git client to time out and return a server error if it takes too long to complete.

We should consider whether we need to pull down the whole history every time there is a new push. Using depth: 1, singleBranch: true on the git clone call seems to reduce the amount of time (in our particular instance) that the push takes and allows it to proceed through proxy processing normally.

Describe the solution you'd like

• Tweak the isomorphic-git call during cloning to the minimum required to produce a working diff.
• Pull down selective history such as default branches only?
• Make it configurable?

Describe alternatives you've considered
Increasing the timeout for the server (we have it deployed behind a load balancer) helps marginally but the full clone can be very expensive with large repos.

Additional context

This is likely a problem that affects outdated branches more than anything. When a commit drifts (is left stale for days, weeks or months), the combination of the long clone time + large diff usually results in server errors.

[kriswest]

+1 - We experienced teething trouble with the clone time as well and had to increase timeouts in our loadbalancer for moderate to large repos such as FDC3. In addition to causing timeouts, long clone times affect usability, particularly for new users.

[jescalada]

@coopernetes @kriswest I've done some testing here, and it seems the pullRemote action (and therefore isomorphic-git's git.clone call never gets executed when cloning a repo for the first time or pulling a previously cloned repo. Is this meant to execute only during pushes?

Changing those parameters on the git.clone call doesn't seem to change the time elapsed for cloning/pulling on my end.

Is there a step-by-step sample I can follow to get pullRemote to execute?

Thanks!

[kriswest]

pullRemote is only in the push chain (in order to facilitate getDiff later), so it does indeed only run on a push. @jescalada if you want to experiment with this add some time measurement either on pullRemote or executeChain, then run a push.

Could add the noTags flag as well I imagine (but presumably that mostly affects the fetch?).

@coopernetes if you've tried this out have you run into any diff issues with depth:1? If its not causing trouble it would seem a quick change to make.

[jescalada]

Some more experimenting with pushes this time, with a small commit and short commit history (in progress):

Before fix

Total push time, before approval 5.338s

pullRemote time, before approval 1.628s

Total push time, after approval 9.575s

pullRemote times, after approval 2.485s + 1.707s

[jescalada]

After fix

Total push time, before approval 2.571s

pullRemote time, before approval 1.053s

Total, push time, after approval 4.883s

pullRemote time, after approval 1.093s + 0.810s

[jescalada]

Adding the singleBranch: true and depth: 1 lines seems to halve the push times. I think there are other ways to keep optimizing this though.

Is there a reason why we call pullRemote 3 times to complete one push? Wouldn't it be most efficient to pull once, store the repo (in .remote) for later use, and maybe if we really need to delete it, just delete it after the push is completed (not at the end of the action chain in clearBareClone as we do right now)?

Right now, we're making a temp repo for each action that goes through pullRemote:

  try {
    action.proxyGitPath = `${dir}/${action.timestamp}`;

    step.log(`Creating folder ${action.proxyGitPath}`);

    if (!fs.existsSync(dir)) {
      fs.mkdirSync(dir);
    }

    if (!fs.existsSync(action.proxyGitPath)) {
      fs.mkdirSync(action.proxyGitPath, 0o755);
    }

    ...

    await git.clone({
      fs,
      http: gitHttpClient,
      url: action.url,
      onAuth: () => ({
        username,
        password,
      }),
      dir: `${action.proxyGitPath}/${action.repoName}`,
    });

And this is always deleted in clearBareClone.ts without any other checks - such as whether the push was approved.

const exec = async (req: any, action: Action): Promise<Action> => {
  const step = new Step('clearBareClone');

  // Recursively remove the contents of ./.remote and ignore exceptions
  fs.rm('./.remote', { recursive: true, force: true }, (err) => {
    if (err) {
      throw err;
    }
    console.log(`.remote is deleted!`);
  });

  action.addStep(step);
  return action;
};

Another question: Why is the repo (.remote) stored redundantly within GitProxy and within the user's PC as well? Isn't there a way to share the repo contents/state between the two to reduce the need to push/pull twice each time? Perhaps this is just how proxies work by design, but I'm wondering if there's a better solution here.

[jescalada]

Based on some of the questions from before, I tried modifying the pullRemote so that it identifies actions by their ID (which is really just their to/from commits) rather than the timestamp. This makes the second pullRemote call execute about twice as fast:

Extra fix (identify/save actions by ID)

Total push time, before approval 2.578s

pullRemote time, before approval 1.055s

Total push time, after approval 4.549s

pullRemote time, after approval 1.018s + 0.613s

Notice how in the last screenshot, pullRemote is calling git.fetch instead of git.clone.

Feel free to check out the code changes in #1189!

[kriswest]

Is there a reason why we call pullRemote 3 times to complete one push? Wouldn't it be most efficient to pull once, store the repo (in .remote) for later use, and maybe if we really need to delete it, just delete it after the push is completed (not at the end of the action chain in clearBareClone as we do right now)?

@JamieSlome mentioned that they'd had to start clearing the remote directories as they store the cached user credentials, which then end up persisting in a temp directory. If you have the dirs persist between first push and second push (with approval) then it'll also up the storage requirements for git proxy (depending on usage), which may not be desirable for some.

Is it 3 times? Once each of first and second push, what's the third?

I think the redundancy between the server and client copies of the checkout maybe unavoidable - in order to compute a diff you need the checkout at present. We could start trying to work with the decompressed push data instead, but I think we'd find ourselves with deltas and not the source to apply them to. I suspect there's not a good solution to that. Hence, were I to change something to try and get the benefit of caching... I'd probably give Git Proxy its own git account and flag telling it its allowed to cache a particular repo using its own account (and then deal with switching account for the push??). However, here's hoping that the changes you've already applied mean we don't need to do that ;-)

I see your change to fetch instead of clone - I don't know why we've got a clone still hanging out at that point for you to stick to. Also what if there are remote changes that need to be pulled, post-fetch? Finally, do we even need the pull and to examine it if we've already got it approved? Perhaps thats where we should focus - skipping steps in the chain if we recognize the push and don't need to calculate the diff etc..?

[kriswest]

(although when developing I've appreciated being able to push multiple times and have the push reinterpreted each time...)

[jescalada]

I've done some more profiling, and it seems that the most time-consuming actions (on push before approval) are:

• pullRemote: 1031ms
• writePack: 210ms
• checkHiddenCommits: 168ms
• getDiff: 78ms

These functions take up around ~1.5s out of ~2.3s total execution time for git push.

There's also a mystery function being called that takes up a fair bit of time (most of the remaining unknown ~0.8s). I tried profiling just about every proxy-related function but couldn't narrow it down, my guess is that it's some http library or perhaps the proxy itself that takes some time to process each request.

I'll investigate further to see what causes this and if we can do anything about it.

[jescalada]

I've managed to narrow down the other time-consuming operation: It's the express-http-proxy sendProxyRequest function. In here, there's an option for streaming instead of chunking, which is slightly faster. What I noticed is that we call sendProxyRequest twice with GET owner/repo-name.git/git-receive-pack, and these calls take up 400~500ms each. Perhaps this can/should be cached?

As for the double execution on the post-approval push, I noticed that adding the following check to proxyFilter fixes it:

  if (req.method !== 'GET' || !req.url.includes('/info/refs')) {
    return true;
  }

This check also seems to prevent the clone/fetch distinction that was happening before (so my previous "fix" is no longer necessary).

The combined improvements seem to reduce total push times to ~1.6s + 2.2s down from ~2.5s + 4.5s:

Pinging @fabiovincenzi in case this is helpful to debug test the double execution fix!