From b7493e440843baac1cc1353ca5b43b337535cc23 Mon Sep 17 00:00:00 2001 From: michalsn Date: Mon, 4 Aug 2025 07:57:02 +0200 Subject: [PATCH 1/5] feat: migrations lock --- app/Config/Migrations.php | 15 + system/Database/MigrationRunner.php | 370 ++++++++++++------ system/Language/en/Migrations.php | 1 + .../Migrations/MigrationRunnerTest.php | 83 ++++ user_guide_src/source/changelogs/v4.7.0.rst | 5 + user_guide_src/source/dbmgmt/migration.rst | 2 + 6 files changed, 360 insertions(+), 116 deletions(-) diff --git a/app/Config/Migrations.php b/app/Config/Migrations.php index 1dec8b9b3a40..eb45e72196c4 100644 --- a/app/Config/Migrations.php +++ b/app/Config/Migrations.php @@ -47,4 +47,19 @@ class Migrations extends BaseConfig * - Y_m_d_His_ */ public string $timestampFormat = 'Y-m-d-His_'; + + /** + * -------------------------------------------------------------------------- + * Enable/Disable Migration Lock + * -------------------------------------------------------------------------- + * + * Locking is disabled by default. + * + * When enabled, it will prevent multiple migration processes + * from running at the same time by using a lock mechanism. + * + * This is useful in production environments to avoid conflicts + * or race conditions during concurrent deployments. + */ + public bool $lock = false; } diff --git a/system/Database/MigrationRunner.php b/system/Database/MigrationRunner.php index fdff86589132..6be022584750 100644 --- a/system/Database/MigrationRunner.php +++ b/system/Database/MigrationRunner.php @@ -14,6 +14,7 @@ namespace CodeIgniter\Database; use CodeIgniter\CLI\CLI; +use CodeIgniter\Database\Exceptions\DatabaseException; use CodeIgniter\Events\Events; use CodeIgniter\Exceptions\ConfigException; use CodeIgniter\Exceptions\RuntimeException; @@ -101,6 +102,17 @@ class MigrationRunner */ protected $tableChecked = false; + /** + * Lock the migration table. + */ + protected bool $lock = false; + + /** + * Tracks whether we have already ensured + * the lock table exists or not. + */ + protected bool $lockTableChecked = false; + /** * The full path to locate migration files. * @@ -135,6 +147,7 @@ public function __construct(MigrationsConfig $config, $db = null) { $this->enabled = $config->enabled ?? false; $this->table = $config->table ?? 'migrations'; + $this->lock = $config->lock ?? false; $this->namespace = APP_NAMESPACE; @@ -161,52 +174,66 @@ public function latest(?string $group = null) $this->ensureTable(); - if ($group !== null) { - $this->groupFilter = $group; - $this->setGroup($group); - } + // Try to acquire lock - exit gracefully if another process is running migrations + if ($this->lock && ! $this->acquireMigrationLock()) { + $message = lang('Migrations.locked'); + $this->cliMessages[] = "\t" . CLI::color($message, 'yellow'); - $migrations = $this->findMigrations(); - - if ($migrations === []) { return true; } - foreach ($this->getHistory((string) $group) as $history) { - unset($migrations[$this->getObjectUid($history)]); - } + try { + if ($group !== null) { + $this->groupFilter = $group; + $this->setGroup($group); + } - $batch = $this->getLastBatch() + 1; + $migrations = $this->findMigrations(); - foreach ($migrations as $migration) { - if ($this->migrate('up', $migration)) { - if ($this->groupSkip === true) { - $this->groupSkip = false; + if ($migrations === []) { + return true; + } - continue; - } + foreach ($this->getHistory((string) $group) as $history) { + unset($migrations[$this->getObjectUid($history)]); + } - $this->addHistory($migration, $batch); - } else { - $this->regress(-1); + $batch = $this->getLastBatch() + 1; - $message = lang('Migrations.generalFault'); + foreach ($migrations as $migration) { + if ($this->migrate('up', $migration)) { + if ($this->groupSkip === true) { + $this->groupSkip = false; - if ($this->silent) { - $this->cliMessages[] = "\t" . CLI::color($message, 'red'); + continue; + } - return false; - } + $this->addHistory($migration, $batch); + } else { + $this->regress(-1); - throw new RuntimeException($message); + $message = lang('Migrations.generalFault'); + + if ($this->silent) { + $this->cliMessages[] = "\t" . CLI::color($message, 'red'); + + return false; + } + + throw new RuntimeException($message); + } } - } - $data = get_object_vars($this); - $data['method'] = 'latest'; - Events::trigger('migrate', $data); + $data = get_object_vars($this); + $data['method'] = 'latest'; + Events::trigger('migrate', $data); - return true; + return true; + } finally { + if ($this->lock) { + $this->releaseMigrationLock(); + } + } } /** @@ -230,45 +257,75 @@ public function regress(int $targetBatch = 0, ?string $group = null) $this->ensureTable(); - $batches = $this->getBatches(); - - if ($targetBatch < 0) { - $targetBatch = $batches[count($batches) - 1 + $targetBatch] ?? 0; - } + // Try to acquire lock - exit gracefully if another process is running migrations + if ($this->lock && ! $this->acquireMigrationLock()) { + $message = lang('Migrations.locked'); + $this->cliMessages[] = "\t" . CLI::color($message, 'yellow'); - if ($batches === [] && $targetBatch === 0) { return true; } - if ($targetBatch !== 0 && ! in_array($targetBatch, $batches, true)) { - $message = lang('Migrations.batchNotFound') . $targetBatch; - - if ($this->silent) { - $this->cliMessages[] = "\t" . CLI::color($message, 'red'); + try { + $batches = $this->getBatches(); - return false; + if ($targetBatch < 0) { + $targetBatch = $batches[count($batches) - 1 + $targetBatch] ?? 0; } - throw new RuntimeException($message); - } + if ($batches === [] && $targetBatch === 0) { + return true; + } - $tmpNamespace = $this->namespace; + if ($targetBatch !== 0 && ! in_array($targetBatch, $batches, true)) { + $message = lang('Migrations.batchNotFound') . $targetBatch; - $this->namespace = null; - $allMigrations = $this->findMigrations(); + if ($this->silent) { + $this->cliMessages[] = "\t" . CLI::color($message, 'red'); - $migrations = []; + return false; + } - while ($batch = array_pop($batches)) { - if ($batch <= $targetBatch) { - break; + throw new RuntimeException($message); } - foreach ($this->getBatchHistory($batch, 'desc') as $history) { - $uid = $this->getObjectUid($history); + $tmpNamespace = $this->namespace; + + $this->namespace = null; + $allMigrations = $this->findMigrations(); + + $migrations = []; + + while ($batch = array_pop($batches)) { + if ($batch <= $targetBatch) { + break; + } + + foreach ($this->getBatchHistory($batch, 'desc') as $history) { + $uid = $this->getObjectUid($history); + + if (! isset($allMigrations[$uid])) { + $message = lang('Migrations.gap') . ' ' . $history->version; + + if ($this->silent) { + $this->cliMessages[] = "\t" . CLI::color($message, 'red'); - if (! isset($allMigrations[$uid])) { - $message = lang('Migrations.gap') . ' ' . $history->version; + return false; + } + + throw new RuntimeException($message); + } + + $migration = $allMigrations[$uid]; + $migration->history = $history; + $migrations[] = $migration; + } + } + + foreach ($migrations as $migration) { + if ($this->migrate('down', $migration)) { + $this->removeHistory($migration->history); + } else { + $message = lang('Migrations.generalFault'); if ($this->silent) { $this->cliMessages[] = "\t" . CLI::color($message, 'red'); @@ -278,36 +335,20 @@ public function regress(int $targetBatch = 0, ?string $group = null) throw new RuntimeException($message); } - - $migration = $allMigrations[$uid]; - $migration->history = $history; - $migrations[] = $migration; } - } - - foreach ($migrations as $migration) { - if ($this->migrate('down', $migration)) { - $this->removeHistory($migration->history); - } else { - $message = lang('Migrations.generalFault'); - if ($this->silent) { - $this->cliMessages[] = "\t" . CLI::color($message, 'red'); + $data = get_object_vars($this); + $data['method'] = 'regress'; + Events::trigger('migrate', $data); - return false; - } + $this->namespace = $tmpNamespace; - throw new RuntimeException($message); + return true; + } finally { + if ($this->lock) { + $this->releaseMigrationLock(); } } - - $data = get_object_vars($this); - $data['method'] = 'regress'; - Events::trigger('migrate', $data); - - $this->namespace = $tmpNamespace; - - return true; } /** @@ -328,60 +369,74 @@ public function force(string $path, string $namespace, ?string $group = null) $this->ensureTable(); - if ($group !== null) { - $this->groupFilter = $group; - $this->setGroup($group); + // Try to acquire lock - exit gracefully if another process is running migrations + if ($this->lock && ! $this->acquireMigrationLock()) { + $message = lang('Migrations.locked'); + $this->cliMessages[] = "\t" . CLI::color($message, 'yellow'); + + return true; } - $migration = $this->migrationFromFile($path, $namespace); - if (empty($migration)) { - $message = lang('Migrations.notFound'); + try { + if ($group !== null) { + $this->groupFilter = $group; + $this->setGroup($group); + } - if ($this->silent) { - $this->cliMessages[] = "\t" . CLI::color($message, 'red'); + $migration = $this->migrationFromFile($path, $namespace); + if ($migration === false) { + $message = lang('Migrations.notFound'); - return false; - } + if ($this->silent) { + $this->cliMessages[] = "\t" . CLI::color($message, 'red'); - throw new RuntimeException($message); - } + return false; + } + + throw new RuntimeException($message); + } - $method = 'up'; - $this->setNamespace($migration->namespace); + $method = 'up'; + $this->setNamespace($migration->namespace); - foreach ($this->getHistory($this->group) as $history) { - if ($this->getObjectUid($history) === $migration->uid) { - $method = 'down'; - $migration->history = $history; - break; + foreach ($this->getHistory($this->group) as $history) { + if ($this->getObjectUid($history) === $migration->uid) { + $method = 'down'; + $migration->history = $history; + break; + } } - } - if ($method === 'up') { - $batch = $this->getLastBatch() + 1; + if ($method === 'up') { + $batch = $this->getLastBatch() + 1; + + if ($this->migrate('up', $migration) && $this->groupSkip === false) { + $this->addHistory($migration, $batch); - if ($this->migrate('up', $migration) && $this->groupSkip === false) { - $this->addHistory($migration, $batch); + return true; + } + + $this->groupSkip = false; + } elseif ($this->migrate('down', $migration)) { + $this->removeHistory($migration->history); return true; } - $this->groupSkip = false; - } elseif ($this->migrate('down', $migration)) { - $this->removeHistory($migration->history); - - return true; - } + $message = lang('Migrations.generalFault'); - $message = lang('Migrations.generalFault'); + if ($this->silent) { + $this->cliMessages[] = "\t" . CLI::color($message, 'red'); - if ($this->silent) { - $this->cliMessages[] = "\t" . CLI::color($message, 'red'); + return false; + } - return false; + throw new RuntimeException($message); + } finally { + if ($this->lock) { + $this->releaseMigrationLock(); + } } - - throw new RuntimeException($message); } /** @@ -817,6 +872,89 @@ public function ensureTable() $this->tableChecked = true; } + /** + * Ensures that we have created our migration + * lock table in the database. + * + * @return string The lock table name + */ + protected function ensureLockTable(): string + { + $lockTable = $this->table . '_lock'; + + if ($this->lockTableChecked || $this->db->tableExists($lockTable)) { + return $lockTable; + } + + $forge = Database::forge($this->db); + + $forge->addField([ + 'id' => [ + 'type' => 'BIGINT', + 'auto_increment' => true, + ], + 'lock_name' => [ + 'type' => 'VARCHAR', + 'constraint' => 255, + 'null' => false, + 'unique' => true, + ], + 'acquired_at' => [ + 'type' => 'INTEGER', + 'null' => false, + ], + ]); + + $forge->addPrimaryKey('id'); + $forge->createTable($lockTable, true); + + $this->lockTableChecked = true; + + return $lockTable; + } + + /** + * Acquire exclusive lock on migrations to prevent concurrent execution + * + * @return bool True if lock was acquired, false if another process holds the lock + */ + protected function acquireMigrationLock(): bool + { + $lockTable = $this->ensureLockTable(); + + try { + $this->db->table($lockTable)->insert([ + 'lock_name' => 'migration_process', + 'acquired_at' => Time::now()->getTimestamp(), + ]); + + return $this->db->insertID() > 0; + } catch (DatabaseException) { + // Lock already exists or other error + return false; + } + } + + /** + * Release migration lock + * + * @return bool True if successfully released, false on error + */ + protected function releaseMigrationLock(): bool + { + $lockTable = $this->ensureLockTable(); + + $result = $this->db->table($lockTable) + ->where('lock_name', 'migration_process') + ->delete(); + + if ($result === false) { + log_message('warning', 'Failed to release migration lock'); + } + + return $result; + } + /** * Handles the actual running of a migration. * diff --git a/system/Language/en/Migrations.php b/system/Language/en/Migrations.php index 5d7f3a86542f..a2e66281aed7 100644 --- a/system/Language/en/Migrations.php +++ b/system/Language/en/Migrations.php @@ -22,6 +22,7 @@ 'gap' => 'There is a gap in the migration sequence near version number: ', 'classNotFound' => 'The migration class "%s" could not be found.', 'missingMethod' => 'The migration class is missing an "%s" method.', + 'locked' => 'Migrations already running in another process. Skipping.', // Migration Command 'migHelpLatest' => "\t\tMigrates database to latest available migration.", diff --git a/tests/system/Database/Migrations/MigrationRunnerTest.php b/tests/system/Database/Migrations/MigrationRunnerTest.php index fbe7cbca05f5..6b0fbd60bd84 100644 --- a/tests/system/Database/Migrations/MigrationRunnerTest.php +++ b/tests/system/Database/Migrations/MigrationRunnerTest.php @@ -490,4 +490,87 @@ protected function resetTables($db = null): void $forge->dropTable($table, true); } } + + public function testLatestWithLockingEnabledSucceeds(): void + { + $this->config->lock = true; + + $runner = new MigrationRunner($this->config); + $runner->setNamespace('Tests\Support\MigrationTestMigrations') + ->clearHistory(); + + $this->assertTrue($runner->latest()); + $this->assertTrue($this->db->tableExists('foo')); + + $this->dontSeeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); + } + + public function testRegressWithLockingEnabled(): void + { + $this->config->lock = true; + + $runner = new MigrationRunner($this->config); + $runner->setNamespace('Tests\Support\MigrationTestMigrations') + ->clearHistory(); + + // First run migrations + $runner->latest(); + $this->assertTrue($this->db->tableExists('foo')); + + // Then regress them + $result = $runner->regress(0); + $this->assertTrue($result); + $this->assertFalse($this->db->tableExists('foo')); + } + + public function testLockAcquisitionAndReleaseBasic(): void + { + $this->config->lock = true; + + $runner = new MigrationRunner($this->config); + + $acquireLock = $this->getPrivateMethodInvoker($runner, 'acquireMigrationLock'); + $releaseLock = $this->getPrivateMethodInvoker($runner, 'releaseMigrationLock'); + + // Should acquire lock successfully + $this->assertTrue($acquireLock()); + $this->seeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); + + // Should release successfully + $this->assertTrue($releaseLock()); + $this->dontSeeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); + } + + public function testLockPreventsConcurrentAccess(): void + { + if ($this->db->DBDriver === 'SQLite3' && $this->db->database === ':memory:') { + $this->markTestSkipped('SQLite3 :memory: is not shared between connections.'); + } + + $this->config->lock = true; + + // Create two runners with separate database connections + $runner1 = new MigrationRunner($this->config, $this->db); + $runner2 = new MigrationRunner($this->config, db_connect(null, false)); + + $acquireLock1 = $this->getPrivateMethodInvoker($runner1, 'acquireMigrationLock'); + $releaseLock1 = $this->getPrivateMethodInvoker($runner1, 'releaseMigrationLock'); + $acquireLock2 = $this->getPrivateMethodInvoker($runner2, 'acquireMigrationLock'); + $releaseLock2 = $this->getPrivateMethodInvoker($runner2, 'releaseMigrationLock'); + + // Runner1 should acquire lock successfully + $this->assertTrue($acquireLock1()); + + // Runner2 should fail to acquire lock while runner1 holds it + $this->assertFalse($acquireLock2()); + + // Release lock with runner1 + $this->assertTrue($releaseLock1()); + + // Now runner2 should be able to acquire lock + $this->assertTrue($acquireLock2()); + + // Clean up + $this->assertTrue($releaseLock2()); + } } diff --git a/user_guide_src/source/changelogs/v4.7.0.rst b/user_guide_src/source/changelogs/v4.7.0.rst index f8ad0f446f13..6ef093803feb 100644 --- a/user_guide_src/source/changelogs/v4.7.0.rst +++ b/user_guide_src/source/changelogs/v4.7.0.rst @@ -76,6 +76,11 @@ Query Builder Forge ----- +Migrations +---------- + +- **MigrationRunner:** Added distributed locking support to prevent concurrent migrations in multi-process environments. Enable with ``Config\Migrations::$lock = true``. + Others ------ diff --git a/user_guide_src/source/dbmgmt/migration.rst b/user_guide_src/source/dbmgmt/migration.rst index 5a4bc0852cee..a9baed5b9c62 100644 --- a/user_guide_src/source/dbmgmt/migration.rst +++ b/user_guide_src/source/dbmgmt/migration.rst @@ -244,6 +244,8 @@ Preference Default Options Description table is always created in the default database group (``$defaultGroup``). **timestampFormat** Y-m-d-His\_ The format to use for timestamps when creating a migration. +**lock** false true / false Enable distributed locking to prevent concurrent migrations + in multi-process environments (e.g., Kubernetes). ==================== ============ ============= ============================================================= *************** From 43e1d5448b72a309d37162032429f4a1814ad400 Mon Sep 17 00:00:00 2001 From: michalsn Date: Mon, 4 Aug 2025 08:00:20 +0200 Subject: [PATCH 2/5] cs fix --- tests/system/Database/Migrations/MigrationRunnerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/system/Database/Migrations/MigrationRunnerTest.php b/tests/system/Database/Migrations/MigrationRunnerTest.php index 6b0fbd60bd84..510c8169fa34 100644 --- a/tests/system/Database/Migrations/MigrationRunnerTest.php +++ b/tests/system/Database/Migrations/MigrationRunnerTest.php @@ -502,7 +502,7 @@ public function testLatestWithLockingEnabledSucceeds(): void $this->assertTrue($runner->latest()); $this->assertTrue($this->db->tableExists('foo')); - $this->dontSeeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); + $this->dontSeeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); } public function testRegressWithLockingEnabled(): void @@ -534,11 +534,11 @@ public function testLockAcquisitionAndReleaseBasic(): void // Should acquire lock successfully $this->assertTrue($acquireLock()); - $this->seeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); + $this->seeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); // Should release successfully $this->assertTrue($releaseLock()); - $this->dontSeeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); + $this->dontSeeInDatabase('migrations_lock', ['lock_name' => 'migration_process']); } public function testLockPreventsConcurrentAccess(): void From 03cf3faa5ebe2cc3d9a35b6f0baa3c07e2568f73 Mon Sep 17 00:00:00 2001 From: michalsn Date: Mon, 4 Aug 2025 08:11:48 +0200 Subject: [PATCH 3/5] update phpstan baseline --- utils/phpstan-baseline/empty.notAllowed.neon | 4 ++-- utils/phpstan-baseline/loader.neon | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/phpstan-baseline/empty.notAllowed.neon b/utils/phpstan-baseline/empty.notAllowed.neon index 9e31db0650ba..e41393a87cb6 100644 --- a/utils/phpstan-baseline/empty.notAllowed.neon +++ b/utils/phpstan-baseline/empty.notAllowed.neon @@ -1,4 +1,4 @@ -# total 231 errors +# total 230 errors parameters: ignoreErrors: @@ -69,7 +69,7 @@ parameters: - message: '#^Construct empty\(\) is not allowed\. Use more strict comparison\.$#' - count: 5 + count: 4 path: ../../system/Database/MigrationRunner.php - diff --git a/utils/phpstan-baseline/loader.neon b/utils/phpstan-baseline/loader.neon index 51e85d7e11f0..6a21c978ea51 100644 --- a/utils/phpstan-baseline/loader.neon +++ b/utils/phpstan-baseline/loader.neon @@ -1,4 +1,4 @@ -# total 2846 errors +# total 2845 errors includes: - argument.type.neon - assign.propertyType.neon From a7111afaef5e45d7f711cb8706103af022eb3ee3 Mon Sep 17 00:00:00 2001 From: michalsn Date: Mon, 4 Aug 2025 18:38:34 +0200 Subject: [PATCH 4/5] add upgrading notes --- user_guide_src/source/installation/upgrade_470.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/user_guide_src/source/installation/upgrade_470.rst b/user_guide_src/source/installation/upgrade_470.rst index 2882870a18f4..10d7aad2d26c 100644 --- a/user_guide_src/source/installation/upgrade_470.rst +++ b/user_guide_src/source/installation/upgrade_470.rst @@ -44,7 +44,8 @@ and it is recommended that you merge the updated versions with your application: Config ------ -- @TODO +- app/Config/Migrations.php + - ``Config\Migrations::$lock`` has been added, with a default value set to ``false``. All Changes =========== @@ -52,4 +53,4 @@ All Changes This is a list of all files in the **project space** that received changes; many will be simple comments or formatting that have no effect on the runtime: -- @TODO +- app/Config/Migrations.php From 6d5e47a33449a6ed524c406f92c9a5515ca458b0 Mon Sep 17 00:00:00 2001 From: michalsn Date: Tue, 5 Aug 2025 18:38:48 +0200 Subject: [PATCH 5/5] apply suggestions from code review --- system/Database/MigrationRunner.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system/Database/MigrationRunner.php b/system/Database/MigrationRunner.php index 6be022584750..b30f5bea3ebb 100644 --- a/system/Database/MigrationRunner.php +++ b/system/Database/MigrationRunner.php @@ -883,6 +883,8 @@ protected function ensureLockTable(): string $lockTable = $this->table . '_lock'; if ($this->lockTableChecked || $this->db->tableExists($lockTable)) { + $this->lockTableChecked = true; + return $lockTable; }