From b82ed13586d6d0355e0765692caecebdc51540cb Mon Sep 17 00:00:00 2001
From: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Date: Tue, 6 Jun 2023 03:27:17 +0200
Subject: [PATCH] Database migrations run in seperate sessions and commit on
 success right away (#1822)

This isolates single migration tasks from each other.
The migration itself is now not atomic anymore but each single migration
now on it's own.
This takes load away from databases, as new sessions have a committed
schema available.

We also disable xorm.cache, as the speed improvements are minor but
invalid cache caused by schema changes did happen already in the past.

---------
Reverts #1817
Closes #1821

---------

Co-authored-by: 6543 <6543@obermui.de>
---
 .../migration/017_remove_machine_col.go       | 13 ++++++++
 server/store/datastore/migration/migration.go | 30 ++++++++++++++-----
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/server/store/datastore/migration/017_remove_machine_col.go b/server/store/datastore/migration/017_remove_machine_col.go
index 2e0b963e8..9c02121ce 100644
--- a/server/store/datastore/migration/017_remove_machine_col.go
+++ b/server/store/datastore/migration/017_remove_machine_col.go
@@ -18,9 +18,22 @@ import (
 	"xorm.io/xorm"
 )
 
+type oldStep017 struct {
+	ID      int64  `xorm:"pk autoincr 'step_id'"`
+	Machine string `xorm:"step_machine"`
+}
+
+func (oldStep017) TableName() string {
+	return "steps"
+}
+
 var removeMachineCol = task{
 	name: "remove-machine-col",
 	fn: func(sess *xorm.Session) error {
+		// make sure step_machine column exists
+		if err := sess.Sync(new(oldStep017)); err != nil {
+			return err
+		}
 		return dropTableColumns(sess, "steps", "step_machine")
 	},
 }
diff --git a/server/store/datastore/migration/migration.go b/server/store/datastore/migration/migration.go
index 6d51179b0..f0bbeb67e 100644
--- a/server/store/datastore/migration/migration.go
+++ b/server/store/datastore/migration/migration.go
@@ -15,6 +15,7 @@
 package migration
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 
@@ -25,7 +26,7 @@ import (
 )
 
 // APPEND NEW MIGRATIONS
-// they are executed in order and if one fails woodpecker will try to rollback and quits
+// they are executed in order and if one fails woodpecker will try to rollback that specific one and quits
 var migrationTasks = []*task{
 	&legacy2Xorm,
 	&alterTableReposDropFallback,
@@ -93,6 +94,8 @@ func initNew(sess *xorm.Session) error {
 }
 
 func Migrate(e *xorm.Engine) error {
+	e.SetDisableGlobalCache(true)
+
 	if err := e.Sync(new(migrations)); err != nil {
 		return err
 	}
@@ -116,26 +119,24 @@ func Migrate(e *xorm.Engine) error {
 		return sess.Commit()
 	}
 
-	if err := runTasks(sess, migrationTasks); err != nil {
-		return err
-	}
-
 	if err := sess.Commit(); err != nil {
 		return err
 	}
 
-	if err := e.ClearCache(allBeans...); err != nil {
+	if err := runTasks(e, migrationTasks); err != nil {
 		return err
 	}
 
+	e.SetDisableGlobalCache(false)
+
 	return syncAll(e)
 }
 
-func runTasks(sess *xorm.Session, tasks []*task) error {
+func runTasks(e *xorm.Engine, tasks []*task) error {
 	// cache migrations in db
 	migCache := make(map[string]bool)
 	var migList []*migrations
-	if err := sess.Find(&migList); err != nil {
+	if err := e.Find(&migList); err != nil {
 		return err
 	}
 	for i := range migList {
@@ -149,9 +150,18 @@ func runTasks(sess *xorm.Session, tasks []*task) error {
 		}
 
 		log.Trace().Msgf("start migration task '%s'", task.name)
+		sess := e.NewSession().NoCache()
+		defer sess.Close()
+		if err := sess.Begin(); err != nil {
+			return err
+		}
 
 		if task.fn != nil {
 			if err := task.fn(sess); err != nil {
+				if err2 := sess.Rollback(); err2 != nil {
+					err = errors.Join(err, err2)
+				}
+
 				if task.required {
 					return err
 				}
@@ -166,6 +176,10 @@ func runTasks(sess *xorm.Session, tasks []*task) error {
 		if _, err := sess.Insert(&migrations{task.name}); err != nil {
 			return err
 		}
+		if err := sess.Commit(); err != nil {
+			return err
+		}
+
 		migCache[task.name] = true
 	}
 	return nil