chenwei791129 opened a new pull request, #9190:
URL: https://github.com/apache/devlake/pull/9190
### 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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
