Skip to content

Commit 23c662e

Browse files
Zettat123Zettat123
authored
Support legacy run/job index-based URLs and refactor migration 326 (#37008)
Follow up #36842 Migration `326` can be prohibitively slow on large instances because it scans and rewrites all commit status target URLs generated by Gitea Actions in the database. This PR refactors migration `326` to perform a partial update instead of rewriting every legacy target URL. The reason for this partial rewrite is that **smaller legacy run/job indexes are the most likely to be ambiguous with run/job ID-based URLs** during runtime resolution, so this change prioritizes that subset while avoiding the cost of rewriting all legacy records. To preserve access to old links, this PR introduces `resolveCurrentRunForView` to handle both ID-based URLs and index-based URLs: - For job pages (`/actions/runs/{run}/jobs/{job}`), it first tries to confirm that the URL is ID-based. It does so by checking whether `{job}` can be treated as an existing job ID in the repository and whether that job belongs to `{run}`. If that match cannot be confirmed, it falls back to treating the URL as legacy `run index + job index`, resolves the corresponding run and job, and redirects to the correct ID-based URL. - When both ID-based and index-based interpretations are valid at the same time, the resolver **prefers the ID-based interpretation by default**. For example, if a repository contains one run-job pair (`run_id=3, run_index=2, job_id=4`), and also another run-job pair (`run_id=1100, run_index=3, job_id=1200, job_index=4`), then `/actions/runs/3/jobs/4` is ambiguous. In that case, the resolver treats it as the ID-based URL by default and shows the page for `run_id=3, job_id=4`. Users can still explicitly force the legacy index-based interpretation with `?by_index=1`, which would resolve the same URL to `/actions/runs/1100/jobs/1200`. - For run summary pages (`/actions/runs/{run}`), it uses a best-effort strategy: by default it first treats `{run}` as a run ID, and if no such run exists in the repository, it falls back to treating `{run}` as a legacy run index and redirects to the ID-based URL. Users can also explicitly force the legacy interpretation with `?by_index=1`. - This summary-page compatibility is best-effort, not a strict ambiguity check. For example, if a repository contains two runs: runA (`id=7, index=3`) and runB (`id=99, index=7`), then `/actions/runs/7` will resolve to runA by default, even though the old index-based URL originally referred to runB. The table below shows how valid legacy index-based target URLs are handled before and after migration `326`. Lower-range legacy URLs are rewritten to ID-based URLs, while higher-range legacy URLs remain unchanged in the database but are still handled correctly by `resolveCurrentRunForView` at runtime. | run_id | run_index | job_id | job_index | old target URL | updated by migration 326 | current target URL | can be resolved correctly | |---|---|---|---|---|---|---|---| | 3 | 2 | 4 | 1 | `/user2/repo2/actions/runs/2/jobs/1` | true | `/user2/repo2/actions/runs/3/jobs/4` | true | | 4 | 3 | 8 | 4 | `/user2/repo2/actions/runs/3/jobs/4` | true | `/user2/repo2/actions/runs/4/jobs/8` | true (without migration 326, this URL will resolve to run(`id=3`)) | | 80 | 20 | 170 | 0 | `/user2/repo2/actions/runs/20/jobs/0` | true | `/user2/repo2/actions/runs/80/jobs/170` | true | | 1500 | 900 | 1600 | 0 | `/user2/repo2/actions/runs/900/jobs/0` | false | `/user2/repo2/actions/runs/900/jobs/0` | true | | 2400 | 1500 | 2600 | 0 | `/user2/repo2/actions/runs/1500/jobs/0` | false | `/user2/repo2/actions/runs/1500/jobs/0` | true | | 2400 | 1500 | 2601 | 1 | `/user2/repo2/actions/runs/1500/jobs/1` | false | `/user2/repo2/actions/runs/1500/jobs/1` | true | For users who already ran the old migration `326`, this change has no functional impact. Their historical URLs are already stored in the ID-based form, and ID-based URLs continue to resolve correctly. For users who have not run the old migration `326`, only a subset of legacy target URLs will now be rewritten during upgrade. This avoids the extreme runtime cost of the previous full migration, while all remaining legacy target URLs continue to work through the web-layer compatibility logic. Many thanks to @wxiaoguang for the suggestions.
1 parent 686d10b commit 23c662e

File tree

12 files changed

+695
-213
lines changed

12 files changed

+695
-213
lines changed

‎models/actions/run.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,16 +322,16 @@ func GetRunByRepoAndID(ctx context.Context, repoID, runID int64) (*ActionRun, er
322322
return &run, nil
323323
}
324324

325-
func GetRunByIndex(ctx context.Context, repoID, index int64) (*ActionRun, error) {
325+
func GetRunByRepoAndIndex(ctx context.Context, repoID, runIndex int64) (*ActionRun, error) {
326326
run := &ActionRun{
327327
RepoID: repoID,
328-
Index: index,
328+
Index: runIndex,
329329
}
330330
has, err := db.GetEngine(ctx).Get(run)
331331
if err != nil {
332332
return nil, err
333333
} else if !has {
334-
return nil, fmt.Errorf("run with index %d %d: %w", repoID, index, util.ErrNotExist)
334+
return nil, fmt.Errorf("run with repo_id %d and index %d: %w", repoID, runIndex, util.ErrNotExist)
335335
}
336336

337337
return run, nil

‎models/actions/run_job.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ import (
1818
"xorm.io/builder"
1919
)
2020

21+
// MaxJobNumPerRun is the maximum number of jobs in a single run.
22+
// https://docs.github.com/en/actions/reference/limits#existing-system-limits
23+
// TODO: check this limit when creating jobs
24+
const MaxJobNumPerRun = 256
25+
2126
// ActionRunJob represents a job of a run
2227
type ActionRunJob struct {
2328
ID int64

‎models/migrations/fixtures/Test_FixCommitStatusTargetURLToUseRunAndJobID/action_run.yml‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,29 @@
11
# type ActionRun struct {
2-
# ID int64 `xorm:"pk autoincr"`
3-
# RepoID int64 `xorm:"index"`
4-
# Index int64
2+
# ID int64 `xorm:"pk autoincr"`
3+
# RepoID int64 `xorm:"index"`
4+
# Index int64
5+
# CommitSHA string `xorm:"commit_sha"`
6+
# Event string
7+
# TriggerEvent string
8+
# EventPayload string `xorm:"LONGTEXT"`
59
# }
610
-
7-
id: 106
8-
repo_id: 1
11+
id: 990
12+
repo_id: 100
913
index: 7
14+
commit_sha: merge-sha
15+
event: pull_request
16+
event_payload: '{"pull_request":{"head":{"sha":"sha-shared"}}}'
17+
-
18+
id: 991
19+
repo_id: 100
20+
index: 8
21+
commit_sha: sha-shared
22+
event: push
23+
event_payload: '{"head_commit":{"id":"sha-shared"}}'
24+
-
25+
id: 1991
26+
repo_id: 100
27+
index: 9
28+
commit_sha: sha-other
29+
event: release

‎models/migrations/fixtures/Test_FixCommitStatusTargetURLToUseRunAndJobID/action_run_job.yml‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,14 @@
33
# RunID int64 `xorm:"index"`
44
# }
55
-
6-
id: 530
7-
run_id: 106
6+
id: 997
7+
run_id: 990
88
-
9-
id: 531
10-
run_id: 106
9+
id: 998
10+
run_id: 990
11+
-
12+
id: 1997
13+
run_id: 991
14+
-
15+
id: 1998
16+
run_id: 1991

‎models/migrations/fixtures/Test_FixCommitStatusTargetURLToUseRunAndJobID/commit_status.yml‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,51 @@
11
# type CommitStatus struct {
22
# ID int64 `xorm:"pk autoincr"`
33
# RepoID int64 `xorm:"index"`
4+
# SHA string
45
# TargetURL string
56
# }
67
-
7-
id: 10
8-
repo_id: 1
8+
id: 10010
9+
repo_id: 100
10+
sha: sha-shared
911
target_url: /testuser/repo1/actions/runs/7/jobs/0
1012
-
11-
id: 11
12-
repo_id: 1
13+
id: 10011
14+
repo_id: 100
15+
sha: sha-shared
1316
target_url: /testuser/repo1/actions/runs/7/jobs/1
1417
-
15-
id: 12
16-
repo_id: 1
18+
id: 10012
19+
repo_id: 100
20+
sha: sha-shared
21+
target_url: /testuser/repo1/actions/runs/8/jobs/0
22+
-
23+
id: 10013
24+
repo_id: 100
25+
sha: sha-other
26+
target_url: /testuser/repo1/actions/runs/9/jobs/0
27+
-
28+
id: 10014
29+
repo_id: 100
30+
sha: sha-shared
1731
target_url: /otheruser/badrepo/actions/runs/7/jobs/0
1832
-
19-
id: 13
20-
repo_id: 1
33+
id: 10015
34+
repo_id: 100
35+
sha: sha-shared
2136
target_url: /testuser/repo1/actions/runs/10/jobs/0
2237
-
23-
id: 14
24-
repo_id: 1
38+
id: 10016
39+
repo_id: 100
40+
sha: sha-shared
2541
target_url: /testuser/repo1/actions/runs/7/jobs/3
2642
-
27-
id: 15
28-
repo_id: 1
43+
id: 10017
44+
repo_id: 100
45+
sha: sha-shared
2946
target_url: https://ci.example.com/build/123
47+
-
48+
id: 10018
49+
repo_id: 100
50+
sha: sha-shared
51+
target_url: /testuser/repo1/actions/runs/990/jobs/997

‎models/migrations/fixtures/Test_FixCommitStatusTargetURLToUseRunAndJobID/commit_status_summary.yml‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
# TargetURL string
77
# }
88
-
9-
id: 20
10-
repo_id: 1
11-
sha: "012345"
12-
state: success
9+
id: 10020
10+
repo_id: 100
11+
sha: sha-shared
12+
state: pending
1313
target_url: /testuser/repo1/actions/runs/7/jobs/0
1414
-
15-
id: 21
16-
repo_id: 1
17-
sha: "678901"
18-
state: success
19-
target_url: https://ci.example.com/build/123
15+
id: 10021
16+
repo_id: 100
17+
sha: sha-other
18+
state: pending
19+
target_url: /testuser/repo1/actions/runs/9/jobs/0

‎models/migrations/fixtures/Test_FixCommitStatusTargetURLToUseRunAndJobID/repository.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
# Name string
55
# }
66
-
7-
id: 1
7+
id: 100
88
owner_name: testuser
99
name: repo1

‎models/migrations/migrations.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ func prepareMigrationTasks() []*migration {
400400
newMigration(323, "Add support for actions concurrency", v1_26.AddActionsConcurrency),
401401
newMigration(324, "Fix closed milestone completeness for milestones with no issues", v1_26.FixClosedMilestoneCompleteness),
402402
newMigration(325, "Fix missed repo_id when migrate attachments", v1_26.FixMissedRepoIDWhenMigrateAttachments),
403-
newMigration(326, "Migrate commit status target URL to use run ID and job ID", v1_26.FixCommitStatusTargetURLToUseRunAndJobID),
403+
newMigration(326, "Partially migrate commit status target URL to use run ID and job ID", v1_26.FixCommitStatusTargetURLToUseRunAndJobID),
404404
newMigration(327, "Add disabled state to action runners", v1_26.AddDisabledToActionRunner),
405405
newMigration(328, "Add TokenPermissions column to ActionRunJob", v1_26.AddTokenPermissionsToActionRunJob),
406406
newMigration(329, "Add unique constraint for user badge", v1_26.AddUniqueIndexForUserBadge),

0 commit comments

Comments
 (0)