Repository navigation
feat(backend): sync git commit metadata into commits tables (#2079) - #2118
felipebergamin wants to merge 13 commits into
Conversation
2185f8e to
93ed169
Compare
* Add `Commits` model: unique `git_commit_hash`, nullable author/committer/subject/message/`fetched_from_url` * Add `CommitParents` model: FKs to `commits.id`, `ord` with 0 = first parent, unique `(commit_id, ord)` * Add migration `0021_commits_and_commit_parents` (`commits`, `commit_parents` tables and indexes) * Join from `checkouts.git_commit_hash` without new checkout columns Signed-off-by: Alan Peixinho <alan.peixinho@profusion.mobi>
Dump commits and commit_parents in full so git ancestry survives restore. Signed-off-by: Alan Peixinho <alan.peixinho@profusion.mobi>
…ci#2089) Keep commits rows narrow: unique git name/email in commit_identity, subject plus UTF-8 body in commit_message as bytea. update_db dumps and restores the new tables and still accepts a flat legacy commits CSV.
93ed169 to
85dcd65
Compare
| @@ -0,0 +1,59 @@ | |||
| --- | |||
There was a problem hiding this comment.
maybe we shoudl keep those skill related files only on the dedicated PR.
| parser.add_argument( | ||
| "--dry-run", | ||
| action="store_true", | ||
| help="Parse as requested, write nothing to the database or tips file.", |
There was a problem hiding this comment.
the dry run still updates with fetch (so it is not totally dry), even if we are not writing to database, might be something worth pointing.
| for metadata in metadatas | ||
| ] | ||
| Commits.objects.bulk_create( | ||
| rows, ignore_conflicts=True, batch_size=UPSERT_BATCH_SIZE |
There was a problem hiding this comment.
we are ignoring on conflicts here, we should opt to either update on conflict or change the upsert name.
85dcd65 to
16397d9
Compare
Add a SHA-only helper for commit author/parents so later sync can reuse parsing without duplicating git format handling. Signed-off-by: Felipe Bergamin <felipebergamin@profusion.mobi>
- Move fetch timeout and max pack size to Django settings (GIT_FETCH_TIMEOUT_SECONDS, GIT_FETCH_MAX_PACK_BYTES) - Document URL preference tiers and why the git CLI is used - Reuse author/committer constants in tests
…nelci#2079) Fetch allowlisted trees into GIT_MIRROR_DIR and ingest the tip delta into commits / commit_parents (not checkout hashes). Probe filter support, keep kernel.org packs, split mirror vs ingest crons, and exclude stored tips with ^sha on rev-list stdin. Signed-off-by: Felipe Bergamin <felipebergamin@profusion.mobi>
Commit objects are immutable, so existing hashes stay as stored. Dry-run help now says the mirror fetch still updates the repo on disk, while ingest does not fetch.
Author and committer now live on commit_identity, and subject and message on commit_message.
80974f5 to
07d64ab
Compare
Nested identity and message stores pushed commit_store over the complexity limit.
Record the mirror's on-disk size when sync_commit_mirror finishes so Prometheus can alert above 10 GB. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Felipe Bergamin <felipebergamin@profusion.mobi>
ef2be10 to
bfa9a45
Compare
|
|
||
| if not dry_run: | ||
| write_stored_tips(repo_dir, list_tips(repo_dir)) | ||
| out("tips recorded; next run only ingests what is new") |
There was a problem hiding this comment.
I am unsure here ... but what happens if we fail on ingestion? could we potentially move the tip and loose some hashes from being ingested again later?
| self.stdout.write("Commits migration completed") | ||
|
|
||
| def restore_commits(self) -> None: | ||
| with TextIOWrapper(self.snapshot_archive.extractfile("commits.csv")) as file: |
There was a problem hiding this comment.
I see we are checking for file existing before opening for the other csv, should we also add it here?
| ).decode() | ||
| except (CommitMetadataError, ValueError) as exc: | ||
| logger.warning("verify-pack %s failed: %s", idx, exc) | ||
| return set() |
There was a problem hiding this comment.
if we have an error, we would return an empty set. Wouldnt it cause the pack_reject_reason to return None (no reason)?
| finally: | ||
| self._record_mirror_size(mirror_dir) | ||
|
|
||
| def _record_mirror_size(self, mirror_dir: Path) -> None: |
There was a problem hiding this comment.
I dont know how much this is realy worth it, but maybe adding an option to run gc aggressive could help gain some space (but it is kinda expensive, so I dont know if this helps much).
This is really some idea being thrown, so, feel free to ignore if it could bring more trouble than it is worth it.
There was a problem hiding this comment.
Did some test on mainline, and got from 3.56 GiB to 2.20 GiB. But it also took more than 30min :/
| severity: warning | ||
| annotations: | ||
| summary: "Git commit mirror is larger than 10 GB" | ||
| description: "GIT_MIRROR_DIR is {{ $value | humanize1024 }}B after the last sync_commit_mirror run. Steady state is about 6–8 GB." |
There was a problem hiding this comment.
this is small, but we are using humanize1024 (base 1024), while our check is using base 1000, maybe we should use 10 * 1024 * 1024 * 1024
Pass --shallow-since only after ingest has caught up, and make sync_commit_gc delete unreachable objects immediately. A failed ingest no longer advances synced-tips. Co-authored-by: Cursor <cursoragent@cursor.com>
A per-tip cat-file asked the promisor remote for commits gc had already deleted, so the mirror fetch stalled before it started. Co-authored-by: Cursor <cursoragent@cursor.com>
| # After that 400, git sits on the dead connection instead of exiting. Give curl | ||
| # a stall deadline, and keep auto-gc from repacking a multi-GB mirror mid-run. | ||
| _git(repo, "config", "http.lowSpeedLimit", "1000") | ||
| _git(repo, "config", "http.lowSpeedTime", "60") |
There was a problem hiding this comment.
I was having some trouble fetching the repo, so I had to increase the lowSpeedTime threshold, might be safe to have higher values.
Summary
Store git commit metadata in Postgres instead of reading it from KCIDB submissions. Author, committer, subject, message, and ordered parents live on
commits,commit_identity,commit_message, andcommit_parents. The same hash can appear on many checkouts, so that data does not live oncheckouts.This PR includes the work previously reviewed in #2113:
commits,commit_identity,commit_message,commit_parents.parse_commiton an existing repo, and a throwaway fetch onGIT_SCRATCH_DIR. No database writes. The mirror ingest reuses that parser.Two daily cron jobs fill those tables from git:
sync_commit_mirror(04:00) fetches the ancestor closure of checkout branches, from thetree-names.yamlallowlist, into one bare repo atGIT_MIRROR_DIR.sync_commit_ingest(10:00) parses new objects from that mirror and inserts them.--fill-gapscovers checkout hashes the mirror still does not have.When the mirror job finishes, it writes
GIT_MIRROR_DIR/mirror-size-bytes. WithPROMETHEUS_METRICS_ENABLED=true, the metrics process exposes:git_mirror_size_bytes— apparent size of the mirror, in bytesgit_mirror_size_mtime_seconds— Unix mtime of that file, sotime() - git_mirror_size_mtime_secondsis how long ago the size was recordedGitMirrorSizeHighfires when the size stays above 10 GB for 30 minutes. Steady state is about 6–8 GB. The series stay absent until the first completed fetch. The alert shows in Prometheus and does not page.Details:
docs/sync-commits.md.Closes #2090
Test plan
poetry run pytest kernelCI_app/tests/unitTests/helpers/commitSync_test.py kernelCI_app/tests/unitTests/helpers/gitCommit_test.py kernelCI_app/tests/unitTests/utils/git_mirror_size_test.pygitCommit_test.pycovers fixture parse (author, committer, subject, ordered parents), parse against an existing repo with no fetch, and typed errors for fetch failure, oversized or treeful packs, and a bad URLpoetry run python manage.py sync_commit_mirror --mirror-dir /tmp/git-mirroragainst a small allowlist/tmp/git-mirror/mirror-size-bytesis a single integer and matches the directory sizePROMETHEUS_METRICS_ENABLED=true, openhttp://localhost:8001/metrics/and checkgit_mirror_size_bytesandgit_mirror_size_mtime_secondspoetry run python manage.py sync_commit_ingest --mirror-dir /tmp/git-mirror --dry-runparses objects and writes nothing--dry-runinserts commits and parent edges, and a second run skips hashes already stored