Skip to content

fix(gitextractor): keep deepening shallow clones past merge boundaries - #9190

Open
chenwei791129 wants to merge 1 commit into
apache:mainfrom
chenwei791129:fix/gitextractor-deepen-until-since
Open

chenwei791129 wants to merge 1 commit into
apache:mainfrom
chenwei791129:fix/gitextractor-deepen-until-since

Conversation

@chenwei791129

Copy link
Copy Markdown

Summary

Fixes a silent, permanent data gap in gitextractor incremental collection. Commits that are clearly inside the collection window can be dropped when a merge request brings in a branch whose head is older than the previous run. After that, no later incremental run picks them up. Details and a standalone reproduction are in #9189.

What goes wrong today

History on the default branch; the previous run started at 12:00, so this run fetches with since = 12:00:

            12:00 = since (previous run start)
                 |
  R ------------ | -- Q ------ P ------ M      main
  (01-01)        |   13:00    14:00    16:00 (merge commit)
   \             |                     /
    F ---------- | --------------------+       feature (head F at 10:00,
   10:00         |                              collected by the previous run)

This run should store Q, P and M. What actually happens:

  1. git fetch --shallow-since=12:00 stops at M. Git decides "shallow or not" per commit: since one parent of M (F) is older than 12:00, git cuts M off from all its parents, so P and Q are not fetched even though they are newer than 12:00.

  2. The single git fetch --deepen=1 adds one generation: P arrives, but its parent Q does not.

    local clone:   P (parent Q missing) -- M        R -- F
    
  3. CollectCommits() skips any commit whose first parent is not in the local clone (skip commit ... because it has no parent commit), so P is not stored. Q was never fetched.

  4. The next run starts from this run's start time, so P and Q are now "old" and are never fetched again.

When a lost commit is a deployment head, refdiff compares the next deployment against an incomplete graph and old PRs inflate DORA Lead Time for Changes.

The change

After the existing --deepen=1, keep fetching more history for as long as the newest shallow boundary commit is still newer than since. Each extra round deepens by twice as many generations as the previous one (1, 2, 4, ...), with a cap of 10 extra rounds.

after --deepen=1:     P -- M              boundary P (14:00) is newer than since
                                          -> deepen by 1, then 2, 4, ... generations
after extra rounds:   R -- Q -- P -- M    newest boundary commit is older than since -> stop

When the loop ends, every boundary commit is older than since, so it was either stored by an earlier run or is outside the collection window (first sync with timeAfter). The existing "no parent commit" skip then only affects those commits, which is the case #7720 introduced it for. The first sync with timeAfter goes through the same path and benefits as well.

Details:

  • The boundary is read from the bare clone's shallow file. Git can list boundary commits there that it did not fetch, and git log fails on those (--ignore-missing does not help for full object ids), so the commits are first filtered with git cat-file --batch-check.
  • Failures in the new step are logged as warnings and never fail the subtask, so the worst case is the current behaviour.
  • Cost: normally zero or a few extra fetches. The depth doubles each round and the cap is 10 rounds (up to 1023 extra generations), so the cost is bounded. Each round deepens all boundary commits, including old branch tips, so busy repositories fetch some extra old history. If the cap is hit, a warning is logged and the behaviour is the same as before this change.
  • Partial clones: with SkipCommitStat, the clone uses --filter=blob:none. In such a clone, looking up a missing object makes git fetch it from origin. The boundary lookup sets GIT_NO_LAZY_FETCH=1 to prevent that where git supports it (git 2.45+ and some backports, for example Debian's 2.39.5). With older git (such as 2.30.2 in the CI builder image) the lookup may trigger that fetch, which uses the same auth and proxy settings as the other fetches.

Not changed

  • Commits that already went missing are not backfilled; a full sync (Full Refresh) recovers them.
  • Commits whose committer date is older than the previous run but that were pushed after it (for example, pushed hours after being committed) are still subject to the time-based --shallow-since window. That is a separate limitation.
  • Data sources with NoShallowClone are not covered. doubleClone removes the intermediate clone, which is the fetch remote, before the deepen steps run. This is pre-existing and also affects the existing --deepen=1.

Does this close any open issues?

Closes #9189

Screenshots

N/A. New unit tests (they need the git CLI, which the CI builder image has):

  • TestIncrementalCloneKeepsCommitsBehindMergeBoundary builds the history above and checks that Q, P, M and Q's parent are all fetched. It fails without the fix (Q should be fetched).
  • TestNewestShallowCommitTimeIgnoresMissingCommits covers a shallow file that lists a commit that was not fetched, and a repo without a shallow file (like a full clone).

Other Information

  • The same issue affects the go-git and libgit2 collectors, because both skip commits without their first parent; the fix is in the shared clone step.
  • The bug reproduces with git 2.39.5 (the git in the apache/devlake:v1.0.3-beta15 image) and 2.56.0. The new tests pass with git 2.30.2 (CI builder image) and 2.39.5 (with -race on 2.39.5).

--shallow-since makes a merge commit shallow when any of its parents is
older than since, which hides newer commits on its other parent. A single
--deepen=1 can leave them without their first parent, so the collectors
skip them and later incremental runs never fetch them again.

After --deepen=1, keep deepening (1, 2, 4, ... generations, at most 10
extra rounds) while the newest shallow boundary commit is newer than
since. Boundary commits that were not fetched are filtered with
cat-file, lazy fetches are disabled where git supports
GIT_NO_LAZY_FETCH, and failures are only logged as warnings.

Closes apache#9189
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 this pull request may close these issues.

[Bug][gitextractor] Incremental collection permanently drops in-range commits behind merge commits

1 participant