Index mirrors on first sync only, not every update

What does this MR do and why?

Pull mirrors were re-indexing themselves on every scheduled update, default every 30 minutes, regardless of whether anything changed upstream. The after_transition started: :finished hook on ProjectImportState enqueued Search::Elastic::CommitIndexerWorker and Search::Zoekt.index_async unconditionally. On GitLab.com this saturated a Zoekt node's task queue.

EE::Git::BranchPushService already enqueues both indexers whenever the default branch actually moves, and mirror updates reach it because ff_merge and branch creation are Gitaly operation RPCs that run hooks and fire PostReceive. So for a mirror past its first sync, the transition hook was a redundant, timer-based duplicate. It's genuinely needed only for the initial clone, since Projects::ImportService populates the repository without running hooks. This MR adds a reindex_on_finish attribute, set in a before_transition from !ever_updated_successfully? before the same hook overwrites last_update_at and last_successful_update_at, and gates both indexers on it in the after_transition. Non-mirror imports never set the attribute, so they're unaffected.

Review surfaced a second, pre-existing bug that this change would otherwise have exposed. An earlier callback, after_transition [:scheduled, :started] => [:finished, :failed], calls Gitlab::Mirror.decrement_capacity, a Redis srem? that returns false when the project id isn't in the capacity set. The state_machines terminator halts the callback chain on any false return, so every later callback — including the indexing hook and geo_handle_after_import_finished — was skipped. Capacity is only added on a later transition, so a true first sync (starting from none) never holds a slot and always hit this halt. On master it was masked by the unconditional re-index this MR removes: a newly imported mirror got indexed about thirty minutes late instead of never. Removing the re-index without fixing the halt would have left imported mirrors permanently missing from code search.

The fix makes the capacity callback end with nil instead of the boolean; decrement_capacity still runs and the Redis removal still happens, only the terminator's read of the return value changes, and nil was already what non-mirror projects returned there. This also means geo_handle_after_import_finished now runs for first-sync mirrors, logging Geo update events for the project, wiki, and design repositories as intended. Verified with two real imports into a Zoekt-enabled namespace: with the fix, a Search::Zoekt::Repository row and an index_repo task are created for the import; without it, neither is. Specs no longer need to prime mirror capacity artificially, so they now exercise the same path production takes.

Note for reviewers: increment_capacity on the inverse transition has the same sadd?-returns-false shape. Nothing is declared after it today, so there is no defect to fix, but it is the same trap if a callback is ever added below it.

References

Screenshots or screen recordings

Not applicable. No user-visible interface changes.

How to set up and validate locally

  1. In a Zoekt-enabled namespace, create a project with Import project from URL and mirroring enabled.
  2. After the import finishes, confirm it was indexed:
    Search::Zoekt::Repository.find_by(project_id: <id>)   # => a row, state: initializing
    On master this returns nil — the halt described above.
  3. Trigger a second mirror update with no upstream changes:
    is = Project.find(<id>).import_state
    is.update!(status: :scheduled, next_execution_timestamp: 1.minute.ago)
    RepositoryUpdateMirrorWorker.new.perform(<id>)
  4. Confirm no additional task was created — before this MR, every such update enqueued one.
  5. Push a commit to the default branch and confirm a task is created, via the EE::Git::BranchPushService path.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

🤖 Generated with Claude Code

Edited by Ravi Kumar

Merge request reports

Loading
Loading