Keep database transactions not too big (#13254)

* Keep database transactions not too big

* Fix #13255

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Only cache the last repo

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Andrew Thornton <art27@cantab.net>

models/migrations/v156.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 
 	"xorm.io/xorm"
@@ -35,9 +36,10 @@ func fixPublisherIDforTagReleases(x *xorm.Engine) error {
 	}
 
 	type Repository struct {
-		ID      int64
+		ID        int64
-		OwnerID int64
+		OwnerID   int64
-		Name    string
+		OwnerName string
+		Name      string
 	}
 
 	type User struct {
@@ -50,27 +52,23 @@ func fixPublisherIDforTagReleases(x *xorm.Engine) error {
 	sess := x.NewSession()
 	defer sess.Close()
 
-	if err := sess.Begin(); err != nil {
-		return err
-	}
-
 	var (
-		gitRepoCache = make(map[int64]*git.Repository)
+		repo    *Repository
-		gitRepo      *git.Repository
+		gitRepo *git.Repository
-		repoCache    = make(map[int64]*Repository)
-		userCache    = make(map[int64]*User)
-		ok           bool
-		err          error
 	)
 	defer func() {
-		for i := range gitRepoCache {
+		if gitRepo != nil {
-			gitRepoCache[i].Close()
+			gitRepo.Close()
 		}
 	}()
 	for start := 0; ; start += batchSize {
 		releases := make([]*Release, 0, batchSize)
 
-		if err := sess.Limit(batchSize, start).Asc("id").Where("is_tag=?", true).Find(&releases); err != nil {
+		if err := sess.Begin(); err != nil {
+			return err
+		}
+
+		if err := sess.Limit(batchSize, start).Asc("repo_id", "id").Where("is_tag=?", true).Find(&releases); err != nil {
 			return err
 		}
 
@@ -79,39 +77,36 @@ func fixPublisherIDforTagReleases(x *xorm.Engine) error {
 		}
 
 		for _, release := range releases {
-			gitRepo, ok = gitRepoCache[release.RepoID]
+			if repo == nil || repo.ID != release.RepoID {
-			if !ok {
+				if gitRepo != nil {
-				repo, ok := repoCache[release.RepoID]
+					gitRepo.Close()
-				if !ok {
+					gitRepo = nil
-					repo = new(Repository)
+				}
-					has, err := sess.ID(release.RepoID).Get(repo)
+				repo = new(Repository)
-					if err != nil {
+				has, err := sess.ID(release.RepoID).Get(repo)
-						return err
+				if err != nil {
-					} else if !has {
+					return err
-						return fmt.Errorf("Repository %d is not exist", release.RepoID)
+				} else if !has {
-					}
+					log.Warn("Release[%d] is orphaned and refers to non-existing repository %d", release.ID, release.RepoID)
-
+					log.Warn("This release should be deleted")
-					repoCache[release.RepoID] = repo
+					continue
 				}
 
-				user, ok := userCache[repo.OwnerID]
+				if repo.OwnerName == "" {
-				if !ok {
+					// v120.go migration may not have been run correctly - we'll just replicate it here
-					user = new(User)
+					// because this appears to be a common-ish problem.
-					has, err := sess.ID(repo.OwnerID).Get(user)
+					if _, err := sess.Exec("UPDATE repository SET owner_name = (SELECT name FROM `user` WHERE `user`.id = repository.owner_id)"); err != nil {
-					if err != nil {
 						return err
-					} else if !has {
-						return fmt.Errorf("User %d is not exist", repo.OwnerID)
 					}
 
-					userCache[repo.OwnerID] = user
+					if _, err := sess.ID(release.RepoID).Get(repo); err != nil {
+						return err
+					}
 				}
-
+				gitRepo, err = git.OpenRepository(repoPath(repo.OwnerName, repo.Name))
-				gitRepo, err = git.OpenRepository(repoPath(user.Name, repo.Name))
 				if err != nil {
 					return err
 				}
-				gitRepoCache[release.RepoID] = gitRepo
 			}
 
 			commit, err := gitRepo.GetTagCommit(release.TagName)
@@ -134,7 +129,11 @@ func fixPublisherIDforTagReleases(x *xorm.Engine) error {
 				return err
 			}
 		}
+
+		if err := sess.Commit(); err != nil {
+			return err
+		}
 	}
 
-	return sess.Commit()
+	return nil
 }