diff --git a/API/StorageHandlerInterface.php b/API/StorageHandlerInterface.php index 7075048c..b23d158c 100644 --- a/API/StorageHandlerInterface.php +++ b/API/StorageHandlerInterface.php @@ -24,6 +24,7 @@ public function loadMigration($migrationName); /** * Creates and stores a new migration (leaving it in TODO status) + * * @param MigrationDefinition $migrationDefinition * @return mixed * @throws \Exception If the migration exists already @@ -32,6 +33,7 @@ public function addMigration(MigrationDefinition $migrationDefinition); /** * Starts a migration (creates and stores it, in STARTED status) + * * @param MigrationDefinition $migrationDefinition * @return Migration * @throws \Exception If the migration was already executed, skipped or executing @@ -40,6 +42,7 @@ public function startMigration(MigrationDefinition $migrationDefinition); /** * Ends a migration (updates it) + * * @param Migration $migration * @param bool $force When true, the migration will be updated even if it was not in 'started' status * @throws \Exception If the migration was not started (unless $force=true) @@ -48,12 +51,14 @@ public function endMigration(Migration $migration, $force=false); /** * Removes a migration from storage (regardless of its status) + * * @param Migration $migration */ public function deleteMigration(Migration $migration); /** * Skips a migration (upserts it, in SKIPPED status) + * * @param MigrationDefinition $migrationDefinition * @return Migration * @throws \Exception If the migration was already executed, skipped or executing diff --git a/Command/MigrateCommand.php b/Command/MigrateCommand.php index 92208395..ca215d4d 100644 --- a/Command/MigrateCommand.php +++ b/Command/MigrateCommand.php @@ -80,17 +80,17 @@ protected function execute(InputInterface $input, OutputInterface $output) $this->setVerbosity(self::VERBOSITY_CHILD); } - $migrationsService = $this->getMigrationService(); + $migrationService = $this->getMigrationService(); $paths = $input->getOption('path'); - $migrationDefinitions = $migrationsService->getMigrationsDefinitions($paths); - $migrations = $migrationsService->getMigrations(); + $migrationDefinitions = $migrationService->getMigrationsDefinitions($paths); + $migrations = $migrationService->getMigrations(); // filter away all migrations except 'to do' ones $toExecute = array(); foreach($migrationDefinitions as $name => $migrationDefinition) { if (!isset($migrations[$name]) || (($migration = $migrations[$name]) && $migration->status == Migration::STATUS_TODO)) { - $toExecute[$name] = $migrationsService->parseMigrationDefinition($migrationDefinition); + $toExecute[$name] = $migrationService->parseMigrationDefinition($migrationDefinition); } } @@ -99,10 +99,10 @@ protected function execute(InputInterface $input, OutputInterface $output) if (empty($paths)) { foreach ($migrations as $migration) { if ($migration->status == Migration::STATUS_TODO && !isset($toExecute[$migration->name])) { - $migrationDefinitions = $migrationsService->getMigrationsDefinitions(array($migration->path)); + $migrationDefinitions = $migrationService->getMigrationsDefinitions(array($migration->path)); if (count($migrationDefinitions)) { $migrationDefinition = reset($migrationDefinitions); - $toExecute[$migration->name] = $migrationsService->parseMigrationDefinition($migrationDefinition); + $toExecute[$migration->name] = $migrationService->parseMigrationDefinition($migrationDefinition); } else { // q: shall we raise a warning here ? } @@ -182,6 +182,7 @@ protected function execute(InputInterface $input, OutputInterface $output) } } + /** @var MigrationDefinition $migrationDefinition */ foreach($toExecute as $name => $migrationDefinition) { // let's skip migrations that we know are invalid - user was warned and he decided to proceed anyway @@ -209,20 +210,46 @@ function ($type, $buffer) { } ); - if (!$process->isSuccessful()) { - $err = $process->getErrorOutput(); + try { + + if (!$process->isSuccessful()) { + throw new \Exception($process->getErrorOutput()); + } + + // There are cases where the separate process dies halfway but does not return a non-zero code. + // That's why we should double-check here if the migration is still tagged as 'started'... + /** @var Migration $migration */ + $migration = $migrationService->getMigration($migrationDefinition->name); + + if (!$migration) { + // q: shall we add the migration to the db as failed? In doubt, we let it become a ghost, disappeared without a trace... + throw new \Exception("After the separate process charged to execute the migration finished, the migration can not be found in the database any more."); + } else if ($migration->status == Migration::STATUS_STARTED) { + $errorMsg = "The separate process charged to execute the migration left it in 'started' state. Most likely it died halfway through execution."; + $migrationService->endMigration(New Migration( + $migration->name, + $migration->md5, + $migration->path, + $migration->executionDate, + Migration::STATUS_FAILED, + ($migration->executionError != '' ? ($errorMsg . ' ' . $migration->executionError) : $errorMsg) + )); + throw new \Exception($errorMsg); + } + + } catch(\Exception $e) { if ($input->getOption('ignore-failures')) { - $output->writeln("\nMigration failed! Reason: " . $err . "\n"); + $output->writeln("\nMigration failed! Reason: " . $e->getMessage() . "\n"); continue; } - $output->writeln("\nMigration aborted! Reason: " . $err . ""); + $output->writeln("\nMigration aborted! Reason: " . $e->getMessage() . ""); return 1; } } else { try { - $migrationsService->executeMigration( + $migrationService->executeMigration( $migrationDefinition, !$input->getOption('no-transactions'), $input->getOption('default-language') diff --git a/Core/MigrationService.php b/Core/MigrationService.php index e35e8bd8..e325653c 100644 --- a/Core/MigrationService.php +++ b/Core/MigrationService.php @@ -86,7 +86,7 @@ public function getMigrationsDefinitions(array $paths = array()) } /** - * Return the list of all the migrations which where executed or attempted so far + * Returns the list of all the migrations which where executed or attempted so far * * @return \Kaliop\eZMigrationBundle\API\Collection\MigrationCollection */ @@ -130,6 +130,16 @@ public function skipMigration(MigrationDefinition $migrationDefinition) return $this->storageHandler->skipMigration($migrationDefinition); } + /** + * Not be called by external users for normal use cases, you should use executeMigration() instead + * + * @param Migration $migration + */ + public function endMigration(Migration $migration) + { + return $this->storageHandler->endMigration($migration); + } + /** * Parses a migration definition, return a parsed definition. * If there is a parsing error, the definition status will be updated accordingly diff --git a/Core/StorageHandler/Database.php b/Core/StorageHandler/Database.php index ebe7956e..bee284bd 100644 --- a/Core/StorageHandler/Database.php +++ b/Core/StorageHandler/Database.php @@ -204,6 +204,7 @@ public function endMigration(Migration $migration, $force=false) /** * Removes a Migration from the table - regardless of its state! + * * @param Migration $migration */ public function deleteMigration(Migration $migration) @@ -214,7 +215,7 @@ public function deleteMigration(Migration $migration) } /** - * Stops a migration by storing it in the db. Migration status can not be 'started' + * Skips a migration by storing it in the db. Migration status can not be 'started' * * @param MigrationDefinition $migrationDefinition * @return Migration diff --git a/WHATSNEW.md b/WHATSNEW.md index 9486b56d..4e21317c 100644 --- a/WHATSNEW.md +++ b/WHATSNEW.md @@ -1,3 +1,9 @@ +Version 2.4.2 +============= + +* Fix: improve detection of failed migrations when using separate processes and the child process dies without traces + + Version 2.4.1 =============