Skip to content

repo-updater: queue changeset syncs when the archived flag changes

Administrator requested to merge aharvey/enqueue-syncs-from-repo-updater into main

Created by: LawnGnome

This PR adds functionality to repo-updater that enqueues a sync of each changeset on a repo that has been archived or unarchived. This is modelled heavily after the existing notification for the permission syncer.

This is technically not actually required — all published changesets on open batch changes will eventually be synced, even if they're in an otherwise terminal state — but implementing this reduces the lag time to update the changeset states. (We don't have any possibility of using webhooks here, so this is our only real bet to improve the latency on this process.)

This is PR 4 of 6 addressing #26820 (closed). (Originally it would have been 4 of 4, but I decided to split it up.)

Review notes

Repo Management

Thanks for looking at this!

I don't think there's anything too hairy here. There's some interface cheese moving to help with the injection of the changeset syncer into watchSyncer, but the interesting part is probably the addition of an Archived field to repos.Diff to signal the relevant repos back, since we can't tell from repos.Diff.Modified whether the archived field changed or not.

There's a synchronous query to list the affected changesets on the hot path at the end of watchSyncer that I think is OK, since the syncer doesn't run that frequently, but if you have concerns on that speak up and I'll make it a goroutine or something. It doesn't really need to be synchronous.

Feel free to ignore the Batch Changes specific parts. (Or don't, if you're interested. I can't tell you what to do.)

Batch Changes

The main concern I have here is that we'll overwhelm the priority queue for changeset syncs. This error gets swallowed and turned into a log message, rather than being anything that bubbles back into repo-updater, but it does feel a bit like this is abusing the priority system. Of course, if we do #26922 in the next sprint, this won't matter for very long, since we can just enqueue the syncs that way.

I'm also happy to hear feedback if people think this is just unnecessary from a user experience perspective.

Future work

The next PR will be to enqueue syncs from the reconciler when it notices that a repo has become archived. That one's spicy.

Test plan

Tested manually with a whole bunch of flipping repos in and out of archived states, and added what I think is pretty good test coverage, if I do say so myself.

Merge request reports