From 2a16128ee5ebdcf77ee5c5495f676bc23e155048 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 24 Feb 2025 11:26:53 -0800 Subject: [PATCH] Add global lock for migrations to make upgrade more safe with multiple replications --- cmd/doctor.go | 5 ++-- cmd/migrate.go | 4 ++-- cmd/migrate_storage.go | 4 ++-- models/db/engine_init.go | 4 ++-- models/migrations/migrations.go | 2 +- routers/common/db.go | 7 +++--- routers/install/install.go | 4 ++-- services/doctor/dbversion.go | 3 ++- services/versioned_migration/migration.go | 24 +++++++++++++++++++ .../migration-test/migration_test.go | 7 +++--- 10 files changed, 46 insertions(+), 18 deletions(-) create mode 100644 services/versioned_migration/migration.go diff --git a/cmd/doctor.go b/cmd/doctor.go index e433f4adc5d3c..52699cc4ddbbe 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -4,6 +4,7 @@ package cmd import ( + "context" "fmt" golog "log" "os" @@ -130,8 +131,8 @@ func runRecreateTable(ctx *cli.Context) error { } recreateTables := migrate_base.RecreateTables(beans...) - return db.InitEngineWithMigration(stdCtx, func(x *xorm.Engine) error { - if err := migrations.EnsureUpToDate(x); err != nil { + return db.InitEngineWithMigration(stdCtx, func(ctx context.Context, x *xorm.Engine) error { + if err := migrations.EnsureUpToDate(ctx, x); err != nil { return err } return recreateTables(x) diff --git a/cmd/migrate.go b/cmd/migrate.go index 459805a76d732..25d8b50c45c61 100644 --- a/cmd/migrate.go +++ b/cmd/migrate.go @@ -7,9 +7,9 @@ import ( "context" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/migrations" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/services/versioned_migration" "github.com/urfave/cli/v2" ) @@ -36,7 +36,7 @@ func runMigrate(ctx *cli.Context) error { log.Info("Log path: %s", setting.Log.RootPath) log.Info("Configuration file: %s", setting.CustomConf) - if err := db.InitEngineWithMigration(context.Background(), migrations.Migrate); err != nil { + if err := db.InitEngineWithMigration(context.Background(), versioned_migration.Migrate); err != nil { log.Fatal("Failed to initialize ORM engine: %v", err) return err } diff --git a/cmd/migrate_storage.go b/cmd/migrate_storage.go index 2e3aba021d2ee..f9ed140395f60 100644 --- a/cmd/migrate_storage.go +++ b/cmd/migrate_storage.go @@ -13,7 +13,6 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" - "code.gitea.io/gitea/models/migrations" packages_model "code.gitea.io/gitea/models/packages" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -21,6 +20,7 @@ import ( packages_module "code.gitea.io/gitea/modules/packages" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/services/versioned_migration" "github.com/urfave/cli/v2" ) @@ -227,7 +227,7 @@ func runMigrateStorage(ctx *cli.Context) error { log.Info("Log path: %s", setting.Log.RootPath) log.Info("Configuration file: %s", setting.CustomConf) - if err := db.InitEngineWithMigration(context.Background(), migrations.Migrate); err != nil { + if err := db.InitEngineWithMigration(context.Background(), versioned_migration.Migrate); err != nil { log.Fatal("Failed to initialize ORM engine: %v", err) return err } diff --git a/models/db/engine_init.go b/models/db/engine_init.go index edca6979342df..7a071fa29b918 100644 --- a/models/db/engine_init.go +++ b/models/db/engine_init.go @@ -105,7 +105,7 @@ func UnsetDefaultEngine() { // When called from the "doctor" command, the migration function is a version check // that prevents the doctor from fixing anything in the database if the migration level // is different from the expected value. -func InitEngineWithMigration(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err error) { +func InitEngineWithMigration(ctx context.Context, migrateFunc func(context.Context, *xorm.Engine) error) (err error) { if err = InitEngine(ctx); err != nil { return err } @@ -122,7 +122,7 @@ func InitEngineWithMigration(ctx context.Context, migrateFunc func(*xorm.Engine) // Installation should only be being re-run if users want to recover an old database. // However, we should think carefully about should we support re-install on an installed instance, // as there may be other problems due to secret reinitialization. - if err = migrateFunc(xormEngine); err != nil { + if err = migrateFunc(ctx, xormEngine); err != nil { return fmt.Errorf("migrate: %w", err) } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 87d674a440999..6c4e702f50e28 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -412,7 +412,7 @@ func ExpectedDBVersion() int64 { } // EnsureUpToDate will check if the db is at the correct version -func EnsureUpToDate(x *xorm.Engine) error { +func EnsureUpToDate(ctx context.Context, x *xorm.Engine) error { currentDB, err := GetCurrentDBVersion(x) if err != nil { return err diff --git a/routers/common/db.go b/routers/common/db.go index 61b331760c429..cb163c867dd32 100644 --- a/routers/common/db.go +++ b/routers/common/db.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting/config" + "code.gitea.io/gitea/services/versioned_migration" "xorm.io/xorm" ) @@ -41,16 +42,16 @@ func InitDBEngine(ctx context.Context) (err error) { return nil } -func migrateWithSetting(x *xorm.Engine) error { +func migrateWithSetting(ctx context.Context, x *xorm.Engine) error { if setting.Database.AutoMigration { - return migrations.Migrate(x) + return versioned_migration.Migrate(ctx, x) } if current, err := migrations.GetCurrentDBVersion(x); err != nil { return err } else if current < 0 { // execute migrations when the database isn't initialized even if AutoMigration is false - return migrations.Migrate(x) + return versioned_migration.Migrate(ctx, x) } else if expected := migrations.ExpectedDBVersion(); current != expected { log.Fatal(`"database.AUTO_MIGRATION" is disabled, but current database version %d is not equal to the expected version %d.`+ `You can set "database.AUTO_MIGRATION" to true or migrate manually by running "gitea [--config /path/to/app.ini] migrate"`, current, expected) diff --git a/routers/install/install.go b/routers/install/install.go index 8544717f657dd..b81a5680d39fe 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/models/db" db_install "code.gitea.io/gitea/models/db/install" - "code.gitea.io/gitea/models/migrations" system_model "code.gitea.io/gitea/models/system" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/password/hash" @@ -37,6 +36,7 @@ import ( auth_service "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/forms" + "code.gitea.io/gitea/services/versioned_migration" "gitea.com/go-chi/session" ) @@ -359,7 +359,7 @@ func SubmitInstall(ctx *context.Context) { } // Init the engine with migration - if err = db.InitEngineWithMigration(ctx, migrations.Migrate); err != nil { + if err = db.InitEngineWithMigration(ctx, versioned_migration.Migrate); err != nil { db.UnsetDefaultEngine() ctx.Data["Err_DbSetting"] = true ctx.RenderWithErr(ctx.Tr("install.invalid_db_setting", err), tplInstall, &form) diff --git a/services/doctor/dbversion.go b/services/doctor/dbversion.go index 2a102b2194fc3..34279a45e7a8c 100644 --- a/services/doctor/dbversion.go +++ b/services/doctor/dbversion.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/migrations" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/services/versioned_migration" ) func checkDBVersion(ctx context.Context, logger log.Logger, autofix bool) error { @@ -21,7 +22,7 @@ func checkDBVersion(ctx context.Context, logger log.Logger, autofix bool) error logger.Warn("Got Error: %v during ensure up to date", err) logger.Warn("Attempting to migrate to the latest DB version to fix this.") - err = db.InitEngineWithMigration(ctx, migrations.Migrate) + err = db.InitEngineWithMigration(ctx, versioned_migration.Migrate) if err != nil { logger.Critical("Error: %v during migration", err) } diff --git a/services/versioned_migration/migration.go b/services/versioned_migration/migration.go new file mode 100644 index 0000000000000..daec89d7c11d4 --- /dev/null +++ b/services/versioned_migration/migration.go @@ -0,0 +1,24 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package versioned_migration //nolint + +import ( + "context" + + "code.gitea.io/gitea/models/migrations" + "code.gitea.io/gitea/modules/globallock" + + "xorm.io/xorm" +) + +func Migrate(ctx context.Context, x *xorm.Engine) error { + // only one instance can do the migration at the same time if there are multiple instances + release, err := globallock.Lock(ctx, "gitea_versioned_migration") + if err != nil { + return err + } + defer release() + + return migrations.Migrate(x) +} diff --git a/tests/integration/migration-test/migration_test.go b/tests/integration/migration-test/migration_test.go index 724e442b90479..1b42f00604cfb 100644 --- a/tests/integration/migration-test/migration_test.go +++ b/tests/integration/migration-test/migration_test.go @@ -5,6 +5,7 @@ package migrations import ( "compress/gzip" + "context" "database/sql" "fmt" "io" @@ -247,7 +248,7 @@ func restoreOldDB(t *testing.T, version string) { } } -func wrappedMigrate(x *xorm.Engine) error { +func wrappedMigrate(ctx context.Context, x *xorm.Engine) error { currentEngine = x return migrations.Migrate(x) } @@ -264,7 +265,7 @@ func doMigrationTest(t *testing.T, version string) { beans, _ := db.NamesToBean() - err = db.InitEngineWithMigration(t.Context(), func(x *xorm.Engine) error { + err = db.InitEngineWithMigration(t.Context(), func(ctx context.Context, x *xorm.Engine) error { currentEngine = x return migrate_base.RecreateTables(beans...)(x) }) @@ -272,7 +273,7 @@ func doMigrationTest(t *testing.T, version string) { currentEngine.Close() // We do this a second time to ensure that there is not a problem with retained indices - err = db.InitEngineWithMigration(t.Context(), func(x *xorm.Engine) error { + err = db.InitEngineWithMigration(t.Context(), func(ctx context.Context, x *xorm.Engine) error { currentEngine = x return migrate_base.RecreateTables(beans...)(x) })