fix(backend): warm up a repository without resetting it [IFC-3345]
Why
When a worker reads a repository's commit log or branch drift and has no local copy, it starts a "warm-up" that clones the repository and asks every other worker to clone it too. The warm-up added in #10836 had three problems, raised in this review comment and described in IFC-3345:
- It reset its new copy to the commit Infrahub has imported. When that commit was no longer on the remote (for example after a force push), the reset failed. The warm-up then stopped before telling other workers to clone, and set the repository's operational status to error. A read-only repository has no periodic sync to repair this, so other workers kept answering
NOT_CLONED. - It could roll back a sync, because the reset moved the local branch back to the imported commit.
- It could hang every git operation on the repository. It held the repository lock while it waited for a clone that another task on the same worker had started, and that clone was waiting for the same lock.
Goal: the warm-up clones the repository and asks every other worker to clone it, without moving any local branch, and nothing after the clone can stop that request.
Non-goals:
- Fixing T107: for a read-only repository, "import last commit" can record a commit it did not import when the local copy is already at the remote head. This change leaves more copies at the remote head, so T107 can happen more often. Its owner is not decided yet (this epic or IFC-2998).
- Making other flows that read a local branch (the staging-to-active merge, a proposed change's repository tests) converge. IFC-3210 owns that.
- No new GraphQL field, no graph write and no migration.
Part of the IFC-3101 commit-visibility feature: task T110 in tasks.md. It must merge into this epic branch before the epic merges into develop.
Jira: IFC-3345
What changed
Behavioral changes
- The warm-up no longer resets any copy. Its own copy, and every copy other workers create from its request, stays at the remote head, the same as every first clone today.
- The warm-up now sends
RefreshGitClonefor both repository kinds. It no longer sends aRefreshGitFetchpinned to the imported commit. - The warm-up sends
RefreshGitCloneright after its clone. Only then does it fetch, and a failed fetch is logged as a warning instead of failing the warm-up. - The warm-up no longer holds the repository lock while it clones. The clone takes that lock itself.
- When a worker receives
RefreshGitCloneand its copy has no fetch time, it now fetches the copy, so the commit log and branch drift show a fetch time on every worker. A failed fetch is logged and does not fail the request. - A copy has no fetch time when it has no
FETCH_HEAD(a clone writes none) or an empty one. Git writes an emptyFETCH_HEADwhen a fetch fails, so the commit log and branch drift now reportfetchedAtas null after a failed fetch, instead of the time of the failed attempt. The next clone request fetches that copy again. (Second commit, from Guillaume's review comment.) - A read-write repository with nothing imported is still not warmed up. Its sync creates the copy.
Implementation notes
- The warm-up and the
RefreshGitClonehandler share one helper,fetch_if_no_fetch_timeinbackend/infrahub/git/state/warm_up.py. It fetches only a copy for whichread_fetched_at(the commit-log reader's own check, inlog_reader.py) returns no time, checks again once it holds the lock, and catches onlyGitCommandError. GitRepositoryWarmUploses itslocationfield. Only the pinned fetch used it. The warm-up has not been released, so nothing outside this repository depends on it.- A read-only warm-up no longer reads the imported commit through the API, because it does not use it.
- The branch-drift query still starts the warm-up for the first branch with something imported. Only the code comment changed: the reason is now the read-write check, not the commit to pin.
What stayed the same
- The shape of the commit-log and branch-drift responses. Only the description of
fetchedAtchanged, to say it is null after a failed fetch (schema/schema.graphqland the frontend's generated types are regenerated). - The
RefreshGitFetchhandler, which this branch does not touch.
How to review
backend/infrahub/git/state/warm_up.py: the new order (clone, broadcast, fetch) and the fetch helper.backend/infrahub/message_bus/operations/git/repository.py: theRefreshGitClonehandler now calls the helper.backend/infrahub/git/state/log_reader.py:read_fetched_at, which returns no time for an emptyFETCH_HEAD.backend/tests/integration/git/test_repository_warm_up.py: the tests listed below.dev/specs/ifc-3101-repo-commit-visibility/: T110 is new, T042 is marked as partly replaced, andcontracts/message_bus.md,data-model.md,research.md,plan.mdandquickstart.mddescribe the new warm-up.
Where extra scrutiny helps
- The deadlock test (
test_warm_up_shares_a_clone_another_handler_started_on_this_worker) uses a test client that pauses before its first GraphQL call, to start a clone from another task at the right moment. With the old code it times out after 60 seconds. It never asserts on elapsed time. - After a failed fetch,
fetchedAtis null even when an earlier fetch succeeded. Git emptiesFETCH_HEADbefore it contacts the remote, so the earlier time cannot be recovered from it. Keeping the last successful fetch time would need Infrahub to record it itself after each fetch; that is not part of this PR.
Declined review findings (from a local cubic review run with the checklists in #10715)
- "Move the
try/finallyin the deadlock test into a yield fixture": thefinallywaits for a task that is part of the scenario under test, bounded byasyncio.wait_for. It restores no global state, which is the case the testing checklist asks a fixture for. - "Check for a missing
FETCH_HEADinstead ofreinitialized" was first declined, then adopted after a later review showed thatreinitializedmisses a copy that another call on the same worker cloned. - "
NEVER_CLONED_LOCATIONis a leftover": the component tests still use it to build the request. - "A successful fetch from a remote with no branches also leaves
FETCH_HEADempty": a copy of a repository with no commits fails the local-copy check, so the commit log reports it as not cloned and never reads its fetch time.
How to test
INFRAHUB_USE_TEST_CONTAINERS=true uv run pytest -n 0 \
backend/tests/integration/git/test_repository_warm_up.py \
backend/tests/component/message_bus/operations/git \
backend/tests/unit/git
Locally: 629 passed. These tests fail without the change:
test_warm_up_clones_at_the_remote_head_and_broadcasts_a_clone: the old warm-up sent a pinnedRefreshGitFetch.test_warm_up_of_a_read_only_repository_whose_imported_commit_left_the_remote: the old warm-up raisedRepositoryError.test_warm_up_shares_a_clone_another_handler_started_on_this_worker: the old warm-up deadlocked and the test timed out.test_warm_up_broadcasts_even_when_the_fetch_failsandtest_the_clone_request_fetches_an_existing_copy_never_fetched: fail against the code before the fetch helper.test_the_clone_request_succeeds_when_the_fetch_fails: fails when theexceptis removed.test_the_clone_request_fetches_a_copy_whose_last_fetch_failedandtest_a_clone_whose_last_fetch_failed_reports_no_fetch_time(intest_commit_log.py): fail against the first commit, which treated an emptyFETCH_HEADas a fetch.
Impact & rollout
- Backward compatibility:
GitRepositoryWarmUpdropslocation. The warm-up flow has not been released. - Performance: each warm-up and each worker that receives its request runs at most one fetch, and only for a copy that has no fetch time. A read-only warm-up makes one API call fewer.
- Config/env changes: none.
- Deployment notes: none. This merges into the epic branch, not into
develop.
Checklist
- Tests added/updated
- Changelog entry added (
uv run towncrier create ...): not needed, because the warm-up has not been released - External docs updated (if user-facing or ops-facing change): not needed
- Internal .md docs updated (internal knowledge and AI code tools knowledge)
- I have reviewed AI generated content