Skip to content

Background task to check for any repos w/ last_error nonempty and update

Administrator requested to merge mlw-repo-updater-check-for-last-error into main

Created by: mollylogue

background task in repo-updater to check for repos w/ nonempty last_error and update

Fixes https://github.com/sourcegraph/sourcegraph/issues/25217

TODOs:

Questions:

  • How often does the repo-updater main function get run? My assumption is that this task will kick off on that cadence and fetch the list of all the error-state repos that need to be updated and update them in the background, but I want to make sure this assumption is correct.
  • Is it okay to use the same Syncer for this task as is used everywhere else? I ask because presumably then all these tasks are publishing to the same Synced channel and I'm not sure if that's okay or not.
  • Should I be concerned that there are errors in the logs when the (now turned private) repo is synced, since it can no longer be found? They don't cause the task to exit or anything, just add extra errors to our logs.

NOTE: I'm adding a super rough screen recording of the behavior with this code. You'll see that I see the repo when it's public, then when I turn it to private and navigate back to it on sourcegraph.com, I still see it. When I check the logs I see that gitserver has run into an error (since it can't find the repo anymore). Then, when I restart to trigger the repo-updater running, now the repo updater finds these gitserver errors in the gitserver_repos table and updates the repo, causing it to be deleted. When I navigate back to the page, you see that now it displays a 404 Repository Not Found as desired.

UPDATE: This is now split into two PRs. https://github.com/sourcegraph/sourcegraph/pull/28554 adds in some changes around the repo.Store code. This PR is based off of that branch for now for ease of review/commenting but should be rebased on main once the other PR is merged.

UPDATE: (12/8/21) I'd like to get https://github.com/sourcegraph/sourcegraph/pull/28655 merged first so that I can rebase and improve the testing of this by removing that asynchronous logic.

Merge request reports