From f099dfa0399e86221d76a9482cbcde0a18080823 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 17 Aug 2023 15:14:39 +0800 Subject: [PATCH 1/5] feat: add version to run --- models/actions/run.go | 12 ++++++++++-- models/actions/run_job.go | 37 ++++++++++++++++++++++--------------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index ab6e319b1cc85..2d65abc51f645 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -43,6 +43,7 @@ type ActionRun struct { EventPayload string `xorm:"LONGTEXT"` TriggerEvent string // the trigger event defined in the `on` configuration of the triggered workflow Status Status `xorm:"index"` + Version int `xorm:"version"` // Status could be updated concomitantly, so an optimistic lock is needed Started timeutil.TimeStamp Stopped timeutil.TimeStamp Created timeutil.TimeStamp `xorm:"created"` @@ -337,7 +338,14 @@ func UpdateRun(ctx context.Context, run *ActionRun, cols ...string) error { if len(cols) > 0 { sess.Cols(cols...) } - _, err := sess.Update(run) + affected, err := sess.Update(run) + if err != nil { + return err + } + if affected == 0 { + return fmt.Errorf("run has changed") + // It's impossible that the run is not found, since Gitea never deletes runs. + } if run.Status != 0 || util.SliceContains(cols, "status") { if run.RepoID == 0 { @@ -358,7 +366,7 @@ func UpdateRun(ctx context.Context, run *ActionRun, cols ...string) error { } } - return err + return nil } type ActionRunIndex db.ResourceIndex diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 0239cc0a85e73..9a87980046fef 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -114,32 +114,39 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col if affected != 0 && util.SliceContains(cols, "status") && job.Status.IsWaiting() { // if the status of job changes to waiting again, increase tasks version. if err := IncreaseTaskVersion(ctx, job.OwnerID, job.RepoID); err != nil { - return affected, err + return 0, err } } if job.RunID == 0 { var err error if job, err = GetRunJobByID(ctx, job.ID); err != nil { - return affected, err + return 0, err } } - jobs, err := GetRunJobsByRunID(ctx, job.RunID) - if err != nil { - return affected, err + { + // Other goroutines may aggregate the status of the run and update it too. + // So we need load the run and its jobs before updating the run. + run, err := GetRunByID(ctx, job.RunID) + if err != nil { + return 0, err + } + jobs, err := GetRunJobsByRunID(ctx, job.RunID) + if err != nil { + return 0, err + } + runStatus := aggregateJobStatus(jobs) + if runStatus.IsDone() { + run.Stopped = timeutil.TimeStampNow() + } + run.Status = runStatus + if err := UpdateRun(ctx, run); err != nil { + return 0, fmt.Errorf("update run %d: %w", run.ID, err) + } } - runStatus := aggregateJobStatus(jobs) - - run := &ActionRun{ - ID: job.RunID, - Status: runStatus, - } - if runStatus.IsDone() { - run.Stopped = timeutil.TimeStampNow() - } - return affected, UpdateRun(ctx, run) + return affected, nil } func aggregateJobStatus(jobs []*ActionRunJob) Status { From 8f208405e24c88b25fce3deebfd91f2a66063046 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 17 Aug 2023 15:18:51 +0800 Subject: [PATCH 2/5] feat: add migration --- models/migrations/migrations.go | 2 ++ models/migrations/v1_21/v272.go | 14 ++++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 models/migrations/v1_21/v272.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 7a126593d1458..87c597b573a0d 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -524,6 +524,8 @@ var migrations = []Migration{ NewMigration("Fix PackageProperty typo", v1_21.FixPackagePropertyTypo), // v271 -> v272 NewMigration("Allow archiving labels", v1_21.AddArchivedUnixColumInLabelTable), + // v272 -> v273 + NewMigration("Add Version to ActionRun table", v1_21.AddVersionToActionRunTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_21/v272.go b/models/migrations/v1_21/v272.go new file mode 100644 index 0000000000000..34c5190697a2f --- /dev/null +++ b/models/migrations/v1_21/v272.go @@ -0,0 +1,14 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_21 //nolint +import ( + "xorm.io/xorm" +) + +func AddVersionToActionRunTable(x *xorm.Engine) error { + type ActionRun struct { + Version int `xorm:"version"` + } + return x.Sync(new(ActionRun)) +} From 6756988c0cea71a56613d13cc26a585056081583 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 17 Aug 2023 15:22:59 +0800 Subject: [PATCH 3/5] docs: add comment --- models/actions/run.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/actions/run.go b/models/actions/run.go index 2d65abc51f645..d8ed4060bed16 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -333,6 +333,9 @@ func GetRunByIndex(ctx context.Context, repoID, index int64) (*ActionRun, error) return run, nil } +// UpdateRun updates a run. +// It requires the inputted run has Version set. +// It will return error if the version is not matched (it means the run has been changed after loaded). func UpdateRun(ctx context.Context, run *ActionRun, cols ...string) error { sess := db.GetEngine(ctx).ID(run.ID) if len(cols) > 0 { From f9ea1cd84241dcef2dbf4dfd479969e84083865c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 17 Aug 2023 15:27:13 +0800 Subject: [PATCH 4/5] fix: improve --- models/actions/run_job.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 9a87980046fef..a2eb804d40dce 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -136,11 +136,10 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col if err != nil { return 0, err } - runStatus := aggregateJobStatus(jobs) - if runStatus.IsDone() { + run.Status = aggregateJobStatus(jobs) + if run.Status.IsDone() { run.Stopped = timeutil.TimeStampNow() } - run.Status = runStatus if err := UpdateRun(ctx, run); err != nil { return 0, fmt.Errorf("update run %d: %w", run.ID, err) } From 93f388c6457ace13be69b28f4762e50606890a00 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 17 Aug 2023 16:01:19 +0800 Subject: [PATCH 5/5] feat: improve --- models/actions/run.go | 2 +- models/actions/run_job.go | 7 +++++-- models/actions/task.go | 8 -------- models/migrations/v1_21/v272.go | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index d8ed4060bed16..18ed447e80214 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -43,7 +43,7 @@ type ActionRun struct { EventPayload string `xorm:"LONGTEXT"` TriggerEvent string // the trigger event defined in the `on` configuration of the triggered workflow Status Status `xorm:"index"` - Version int `xorm:"version"` // Status could be updated concomitantly, so an optimistic lock is needed + Version int `xorm:"version default 0"` // Status could be updated concomitantly, so an optimistic lock is needed Started timeutil.TimeStamp Stopped timeutil.TimeStamp Created timeutil.TimeStamp `xorm:"created"` diff --git a/models/actions/run_job.go b/models/actions/run_job.go index a2eb804d40dce..1da58bb65922d 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -137,10 +137,13 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col return 0, err } run.Status = aggregateJobStatus(jobs) - if run.Status.IsDone() { + if run.Started.IsZero() && run.Status.IsRunning() { + run.Started = timeutil.TimeStampNow() + } + if run.Stopped.IsZero() && run.Status.IsDone() { run.Stopped = timeutil.TimeStampNow() } - if err := UpdateRun(ctx, run); err != nil { + if err := UpdateRun(ctx, run, "status", "started", "stopped"); err != nil { return 0, fmt.Errorf("update run %d: %w", run.ID, err) } } diff --git a/models/actions/task.go b/models/actions/task.go index b31afb2126be6..69f52cf0846cf 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -317,14 +317,6 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask return nil, false, nil } - if job.Run.Status.IsWaiting() { - job.Run.Status = StatusRunning - job.Run.Started = now - if err := UpdateRun(ctx, job.Run, "status", "started"); err != nil { - return nil, false, err - } - } - task.Job = job if err := commiter.Commit(); err != nil { diff --git a/models/migrations/v1_21/v272.go b/models/migrations/v1_21/v272.go index 34c5190697a2f..a729c49f1bc71 100644 --- a/models/migrations/v1_21/v272.go +++ b/models/migrations/v1_21/v272.go @@ -8,7 +8,7 @@ import ( func AddVersionToActionRunTable(x *xorm.Engine) error { type ActionRun struct { - Version int `xorm:"version"` + Version int `xorm:"version default 0"` } return x.Sync(new(ActionRun)) }