[mdelacruz-avantio]

Summary

Same change as apache#9281, carried on our release line until an upstream release contains it.

batchFetchDeployments loaded the whole cicd_deployment_commits x commits_diffs join into a slice of full CicdDeploymentCommit rows before keeping the first row per merge commit. On our instance that join returns about 936k rows for 14k merge commits, the lake container peaked at about 1.7 GiB in this subtask and was OOMKilled on every nightly run.

Item	Change
Phase 2 query	Read through db.Cursor and db.Fetch one row at a time into a struct with id, finished_date and merge_sha, keeping the first row per merge commit. Joins, filters and ordering are unchanged
Phase 1 query	Selects dc.id, dc.finished_date, dc.commit_sha instead of dc.*
Error handling	cursor.Err() is checked after the loop
Map contract	The function comment states that map values carry only Id and FinishedDate
e2e	New tie case: a merge commit in the diff range of two production deployments where the lower id started later, expected the earlier-started one

Checks

• Build, vet and gofmt clean. Dora e2e passes on MySQL 8.4 and PostgreSQL 18.4.
• Results are identical: same query, same ordering, same first-wins rule, phase 1 still takes precedence.

Rollout

Image v1.0.3-beta18-avantio.2 is built from the merged tip of this branch and pinned in k8s-infra apps/devlake/values/values.yaml by tag and digest.

[pballester]

/review

[avantio-onyx-agent]

Summary

The PR changes the phase-2 query in batchFetchDeployments so it reads the cicd_deployment_commits × commits_diffs join one row at a time with db.Cursor/db.Fetch into a three-column struct, instead of loading every full CicdDeploymentCommit row into a slice. It also trims phase 1 to id, finished_date, commit_sha. A new e2e case checks that the earliest-started deployment wins over the lower id. The result logic is unchanged and the fix is correct.

Verdict: approve

Findings

Minor

• backend/plugins/dora/tasks/change_lead_time_calculator.go:337-340: map values are now partial CicdDeploymentCommit structs: phase 2 fills only Id/FinishedDate, and phase 1 also has CommitSha. Today this is safe, because the only consumer (lines 136-142) reads just deployment.FinishedDate and deployment.Id, and the function comment at lines 282-283 documents this. Returning a small dedicated type such as map[string]*diffDeployment would let the compiler enforce that contract instead of relying on a comment. This is optional.
• backend/plugins/dora/e2e/change_lead_time/* (pr8 tie case): this test would also pass against the old db.All code, since ordering and first-wins are unchanged. So it guards the started_date-before-id ordering against regressions, not the streaming change itself. That's fine as a parity check, just worth knowing.

Assessment

I checked the change closely:

• Ordering and first-wins: the cursor loop keeps the same ORDER BY cd.commit_sha, dc.started_date ASC, dc.id ASC, the same first-wins rule, and phase 1 still takes precedence.
• Cursor handling: defer cursor.Close() and the cursor.Err() check are both in place.
• Scanning: Dalgorm.Fetch uses ScanRows, which maps id/finished_date/merge_sha onto diffDeployment correctly.
• New fixture: the pr8 values work out: deploy time 66 min (merged 07:00, deployed 08:06) and cycle time 1440+60+66=1566. Deployment 19 is reachable through cicd2→project1 with prev deployment 13 on commit13.

It's ready to merge.

✅ Approved by Onyx Agent — verdict approve, no Critical or Important findings.

---

Review by Onyx Agent — react with 👍 / 👎 to help it improve.