Skip to content

Commit 9964c74

Browse files
GustedGusted
authored andcommitted
feat(perf): speedup available jobs query for user/org runner (#13666)
- The current query is not incorrect or has a inherent performance problem, but MariaDB choses a very wrong query plan for the `action_run_job`, `IDX_action_run_job_repo_id` over `IDX_action_run_job_status`. On surface level the former index looks like a good choice, for a organisation like Forgejo with many already executed jobs, it doesn't become a very effective index. The status index filters very well, because there aren't that many that have as status "waiting". - Relevant: forgejo/forgejo!13658 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13666 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
1 parent 40c9bd6 commit 9964c74

2 files changed

Lines changed: 8 additions & 5 deletions

File tree

models/actions/task.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,8 @@ func GetAvailableJobsForRunner(e db.Engine, runner *ActionRunner) ([]*ActionRunJ
313313
if runner.RepoID != 0 {
314314
jobCond = builder.Eq{"repo_id": runner.RepoID}
315315
} else if runner.OwnerID != 0 {
316-
jobCond = builder.In("repo_id", builder.Select("`repository`.id").From("repository").
317-
Where(builder.Eq{"`repository`.owner_id": runner.OwnerID}))
316+
jobCond = builder.Exists(builder.Select("`repository`.id").From("repository").
317+
Where(builder.Expr("`repository`.owner_id = ? AND repo_id = `repository`.id", runner.OwnerID)))
318318
}
319319

320320
// Concurrency group checks for queuing one run behind the last run in the concurrency group are more

tests/integration/actions_concurrency_group_queue_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ func TestActionConcurrencyRunnerFiltering(t *testing.T) {
5353
},
5454
} {
5555
t.Run(tc.runnerName, func(t *testing.T) {
56-
doTest := func() {
56+
doTest := func(t *testing.T) {
57+
t.Helper()
5758
e := db.GetEngine(t.Context())
5859

5960
runner := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{Name: tc.runnerName})
@@ -68,13 +69,15 @@ func TestActionConcurrencyRunnerFiltering(t *testing.T) {
6869
}
6970

7071
t.Run("ConcurrencyGroupQueueEnabled", func(t *testing.T) {
72+
defer tests.PrintCurrentTest(t)()
7173
defer test.MockVariableValue(&setting.Actions.ConcurrencyGroupQueueEnabled, true)()
72-
doTest()
74+
doTest(t)
7375
})
7476

7577
t.Run("ConcurrencyGroupQueueDisabled", func(t *testing.T) {
78+
defer tests.PrintCurrentTest(t)()
7679
defer test.MockVariableValue(&setting.Actions.ConcurrencyGroupQueueEnabled, false)()
77-
doTest()
80+
doTest(t)
7881
})
7982
})
8083
}

0 commit comments

Comments
 (0)