From 9b40db2aa4a11713bfa40c0ba465116d2d0b3f5e Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Fri, 26 Jun 2026 14:55:06 +0200 Subject: [PATCH] fix: prevent server error when multiple runners fetch same job concurrently on MariaDB 11.8+ (#13204) #12768 added a test `TestCreateTaskForRunnerNoJobUpdated` which has been failing on MariaDB 11.8 in integration-testing. As a fix here, the error occurring (which xorm [recognizes as an `ErrDeadlock`](https://code.forgejo.org/xorm/xorm/src/commit/9eb644730c3ae411837e3a605e73a408ccf9039a/dialects/mysql.go#L961-L972)) is treated the same as having zero records updated, and the test case then passes in MariaDB 11.8. There is no real-world report of "server error when multiple runners fetch same job on MariaDB 11.8+", but it is the expected failure that would occur in production, rarely, that this test failure highlights. ### Tests for Go changes - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - **Already present and failing** - I ran... - [x] `make pr-go` before pushing Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13204 Reviewed-by: Andreas Ahlenstorf --- models/actions/task.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/models/actions/task.go b/models/actions/task.go index 545d320d78..4517f01816 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -19,6 +19,7 @@ import ( "forgejo.org/modules/util" "code.forgejo.org/forgejo/runner/v12/act/jobparser" + "code.forgejo.org/xorm/xorm" lru "github.com/hashicorp/golang-lru/v2" "xorm.io/builder" ) @@ -444,7 +445,14 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner, requestKey, job.TaskID = task.ID // We never have to send a notification here because the job is started with a not done status. - if n, err := UpdateRunJobWithoutNotification(ctx, job, builder.Eq{"task_id": 0}); err != nil { + // + // ErrDeadlock can occur on MariaDB w/ `innodb_snapshot_isolation`, rather than returning 0 records -- we can treat + // that just the same and return the `ErrNoJobUpdated` error code. An alternative would be to use READ COMMITTED + // transaction isolation level, but models/db doesn't currently expose that, and it would cause transaction nesting + // difficulties. + if n, err := UpdateRunJobWithoutNotification(ctx, job, builder.Eq{"task_id": 0}); err != nil && errors.Is(err, xorm.ErrDeadlock) { + return nil, ErrNoJobUpdated + } else if err != nil { return nil, err } else if n != 1 { return nil, ErrNoJobUpdated