Skip to content

LSIF: Do not ensure revision for LSIF result commit resolution

Warren Gifford requested to merge lsif-resolvers-no-ensure-revision into master

Created by: efritz

This fixes the following scenario:

  1. A user uploads an LSIF dump for some repo R at commit C. The upload proxy ensures this commit exists and proxies the upload to the lsif-server (we can hold the invariant that if an LSIF upload exists, the commit existed at some point in time on gitserver).
  2. The user force-pushes such that commit C is no longer on the codehost.
  3. The LSIF-server returns a result with commit C in a reference result (or similar query). This includes a Repository.Commit resolver to fire, which will try to fetch from the codehost when the commit isn't in gitserver.

This operation is too expensive for this particular code path and should simply fail-fast on this edge case of the resolution. After chatting with @keegancsmith and @mrnugget a bit, it seems like it's safe to say that this commit will exist on gitserver if it had existed on gitserver in the past and it also exists on the codehost.

Before this change such conditions cause a multi-second GraphQL query in the API explorer. After this change the results are filtered without a noticeable delay.

Reviwers: This PR simply threads additional variables to expose the NoEnsureCommit option to the RepositoryResolver.Commit method. If there is a cleaner solution I had not found, please suggest it!

Merge request reports

Loading