Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repository shows up as empty when "disableAutoGitUpdates" is true #3336

Closed
beyang opened this issue Apr 10, 2019 · 8 comments · Fixed by #3358
Closed

Repository shows up as empty when "disableAutoGitUpdates" is true #3336

beyang opened this issue Apr 10, 2019 · 8 comments · Fixed by #3358
Assignees
Milestone

Comments

@beyang
Copy link
Member

beyang commented Apr 10, 2019

Repro steps:
• Set "disableGitAutoUpdates": true
• Add Gitolite external service w/ some repositories
• Visit repo on Sourcegraph, observe "Empty repository"
• Go to repo settings mirroring page, observe an error. (After merging #3334, you'll see the error below. Before merging that, the error you see is a JSON unmarshalling error.)

image

I can fix the issue by setting "disableGitAutoUpdates": false

The specific error message is the error gitserver returns, because the repo directory does not yet exist. (It's trying to cd into that directory to find the clone URL, but it doesn't exist.) In theory, gitserver would kick off a git clone in that scenario, but it doesn't appear to.

Investigation

  • This is the method that generates the error shown in the UI. It errors because it attempts to compute the repo remote URL by cd'ing into a directory (the gitserver directory for the repo) that doesn't exist.
  • This enqueue-repo-update handler is invoked to kick off a clone on gitserver. It never fires when disableAutoGitUpdates is true.
  • This is a code path that hits enqueue-repo-update regardless, but it only activates when the repo already exists in gitserver. It's possible that this should fire regardless of whether the repo has already been cloned.
@beyang beyang changed the title [WIP] Repository shows up as empty when "disableAutoGitUpdates" is true Repository shows up as empty when "disableAutoGitUpdates" is true Apr 10, 2019
@beyang
Copy link
Member Author

beyang commented Apr 10, 2019

@keegancsmith @tsenart I've looked into this a fair bit (debug branch bl/3.2-patch), but there are pieces I'm missing about how repo-updater is supposed to work / what its contract should be / what triggers gitserver to clone a repo. Empirically, it appears to be the case that directly visiting a repo page no longer triggers a clone if the repo doesn't already exist on gitserver. Is that expected behavior?

If this makes sense for one of you to own, let me know; otherwise, I'll continue looking into this tomorrow.

@tsenart
Copy link
Contributor

tsenart commented Apr 10, 2019

Empirically, it appears to be the case that directly visiting a repo page no longer triggers a clone if the repo doesn't already exist on gitserver. Is that expected behavior?

When disableGitAutoUpdates is set to true, repo-updater doesn't trigger any Git updates, either via periodic syncing or through explicit visits to the repo. Sorry, this is incorrect. Git updates should still be triggered via EnqueueRepoUpdate calls.

@beyang
Copy link
Member Author

beyang commented Apr 10, 2019

More questions:

  • Where do we expect EnqueueRepoUpdate to be called from? Can it be called from anywhere?
  • What is the source of truth for a repo's clone URL and is this info accessible from anywhere outside the repo-updater loop? Whose job is it to map a repo name to a repo clone URL if the repo is not yet cloned on gitserver?
  • What are the main pieces of state that repo-updater maintains (e.g., repo queue)? Is there a way to tell what the current state is?
  • There are a number of places where DisableAutoGitUpdates is referenced. Its documentation says "Disable periodically fetching git contents for existing repositories" but it appears it may do more than that?
  • I tried repro'ing with GitHub repositories (instead of Gitolite). AFAICT, the bug exists there, but is mitigated by the fact that the GitHub repo is eventually cloned in the background. But doesn't this violate "DisableAutoGitUpdates": true?

@beyang
Copy link
Member Author

beyang commented Apr 11, 2019

The difference appears to be that the clone URL for repos is only made available for uncloned GitHub repos, but not Gitolite ones...

@tsenart
Copy link
Contributor

tsenart commented Apr 11, 2019

@beyang: Answered your questions in the PR.

@keegancsmith
Copy link
Member

@beyang Are there any answers missing from the PR?

Meta: What is a good place to store this sort of information which is longer lived, and documents a poor architecture we plan on improving.

@tsenart
Copy link
Contributor

tsenart commented Apr 11, 2019

Meta: What is a good place to store this sort of information which is longer lived, and documents a poor architecture we plan on improving.

I think the issue related to cleaning up this stuff would be a good place.

@beyang
Copy link
Member Author

beyang commented Apr 11, 2019

Meta: What is a good place to store this sort of information which is longer lived, and documents a poor architecture we plan on improving.

For things pertaining to the internal architecture of the repo-updater service, I'd say long-term place would be package doc of the cmd/repo-updater package. For architecture of how repo-updater and other services interact, top-level dev README maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants