mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-07-13 04:58:46 +00:00
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`](9eb644730c/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 <aahlenst@noreply.codeberg.org>
This commit is contained in:
parent
939e5e8b0a
commit
9b40db2aa4
1 changed files with 9 additions and 1 deletions
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue