From 9fb649793de65a598615c542861281ff15a60439 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Wed, 10 Aug 2016 14:32:00 +0200 Subject: [PATCH 01/13] [ticket/14742] Pause after each schema change It is certainly better than running them all at once PHPBB3-14742 --- phpBB/language/en/migrator.php | 1 + phpBB/phpbb/db/migrator.php | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php index 244a5faadf..7623636941 100644 --- a/phpBB/language/en/migrator.php +++ b/phpBB/language/en/migrator.php @@ -50,6 +50,7 @@ $lang = array_merge($lang, array( 'MIGRATION_NOT_FULFILLABLE' => 'The migration "%1$s" is not fulfillable, missing migration "%2$s".', 'MIGRATION_NOT_VALID' => '%s is not a valid migration.', 'MIGRATION_SCHEMA_DONE' => 'Installed Schema: %1$s; Time: %2$.2f seconds', + 'MIGRATION_SCHEMA_IN_PROGRESS' => 'Installing Schema: %1$s; Time: %2$.2f seconds', 'MIGRATION_SCHEMA_RUNNING' => 'Installing Schema: %s.', 'MIGRATION_INVALID_DATA_MISSING_CONDITION' => 'A migration is invalid. An if statement helper is missing a condition.', diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 7fc3e787e2..739959d7b7 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -308,7 +308,14 @@ class migrator $state['migration_data_state'] = ($result === true) ? '' : $result; $state['migration_schema_done'] = ($result === true); - $this->output_handler->write(array('MIGRATION_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + if ($state['migration_schema_done']) + { + $this->output_handler->write(array('MIGRATION_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + } + else + { + $this->output_handler->write(array('MIGRATION_SCHEMA_IN_PROGRESS', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_VERY_VERBOSE); + } } else if (!$state['migration_data_done']) { @@ -493,8 +500,10 @@ class migrator try { // Result will be null or true if everything completed correctly + // After any schema update step we allow to pause, since + // database changes can take quite some time $result = $this->run_step($step, $last_result, $revert); - if ($result !== null && $result !== true) + if ($result !== null && $result !== true && strpos($step[0], 'dbtools') !== 0) { return serialize(array( 'result' => $result, From 8e1461ca61e3f452935a1253d3afe65e7322d6bc Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Wed, 10 Aug 2016 14:55:39 +0200 Subject: [PATCH 02/13] [ticket/14742] Avoid loop while reverting data This combines reverted updata_data and revert_data into a single array. PHPBB3-14742 --- phpBB/phpbb/db/migration/helper.php | 32 +++++++++++++++++++ phpBB/phpbb/db/migration/tool/config.php | 5 +++ phpBB/phpbb/db/migration/tool/config_text.php | 5 +++ phpBB/phpbb/db/migration/tool/module.php | 5 +++ phpBB/phpbb/db/migration/tool/permission.php | 5 +++ phpBB/phpbb/db/migrator.php | 23 +++++++------ 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/phpBB/phpbb/db/migration/helper.php b/phpBB/phpbb/db/migration/helper.php index e40deeb37b..bce2efff51 100644 --- a/phpBB/phpbb/db/migration/helper.php +++ b/phpBB/phpbb/db/migration/helper.php @@ -81,4 +81,36 @@ class helper return $steps; } + + /** + * Reverse the update steps from an array of data changes + * + * 'If' statements and custom methods will be skipped, for all + * other calls the reverse method of the tool class will be called + * + * @param array $steps Update changes from migration + * + * @return array + */ + public function reverse_update_data($steps) + { + $reversed_array = array(); + + foreach ($steps as $step) + { + $parts = explode('.', $step[0]); + $parameters = $step[1]; + + $class = $parts[0]; + $method = isset($parts[1]) ? $parts[1] : false; + + if ($class !== 'if' && $class !== 'custom') + { + array_unshift($parameters, $method); + $reversed_array[] = array($class . '.reverse', $parameters); + } + } + + return array_reverse($reversed_array); + } } diff --git a/phpBB/phpbb/db/migration/tool/config.php b/phpBB/phpbb/db/migration/tool/config.php index f93e7118c4..2a76409db5 100644 --- a/phpBB/phpbb/db/migration/tool/config.php +++ b/phpBB/phpbb/db/migration/tool/config.php @@ -150,6 +150,11 @@ class config implements \phpbb\db\migration\tool\tool_interface $arguments[0], ); break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migration/tool/config_text.php b/phpBB/phpbb/db/migration/tool/config_text.php index bf8ac55023..21b8a8b3a0 100644 --- a/phpBB/phpbb/db/migration/tool/config_text.php +++ b/phpBB/phpbb/db/migration/tool/config_text.php @@ -115,6 +115,11 @@ class config_text implements \phpbb\db\migration\tool\tool_interface $arguments[] = ''; } break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migration/tool/module.php b/phpBB/phpbb/db/migration/tool/module.php index 035625b095..a6baf40dc1 100644 --- a/phpBB/phpbb/db/migration/tool/module.php +++ b/phpBB/phpbb/db/migration/tool/module.php @@ -454,6 +454,11 @@ class module implements \phpbb\db\migration\tool\tool_interface case 'remove': $call = 'add'; break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php index ceff6d7d5a..3a1e2e344b 100644 --- a/phpBB/phpbb/db/migration/tool/permission.php +++ b/phpBB/phpbb/db/migration/tool/permission.php @@ -637,6 +637,11 @@ class permission implements \phpbb\db\migration\tool\tool_interface $arguments[0], ); break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 739959d7b7..3e69d9613e 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -423,19 +423,11 @@ class migrator if ($state['migration_data_done']) { - if ($state['migration_data_state'] !== 'revert_data') - { - $result = $this->process_data_step($migration->update_data(), $state['migration_data_state'], true); + $steps = array_merge($this->helper->reverse_update_data($migration->update_data()), $migration->revert_data()); + $result = $this->process_data_step($steps, $state['migration_data_state']); - $state['migration_data_state'] = ($result === true) ? 'revert_data' : $result; - } - else - { - $result = $this->process_data_step($migration->revert_data(), '', false); - - $state['migration_data_state'] = ($result === true) ? '' : $result; - $state['migration_data_done'] = ($result === true) ? false : true; - } + $state['migration_data_state'] = ($result === true) ? '' : $result; + $state['migration_data_done'] = ($result === true) ? false : true; $this->set_migration_state($name, $state); } @@ -596,6 +588,13 @@ class migrator throw new \phpbb\db\migration\exception('MIGRATION_INVALID_DATA_MISSING_STEP', $step); } + if ($reverse) + { + // We might get unexpected results when trying + // to revert this, so just avoid it + return false; + } + $condition = $parameters[0]; if (!$condition) From a277f9cf07f73e5f4563d248d1bb4eed49a7e4c9 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Wed, 10 Aug 2016 14:57:33 +0200 Subject: [PATCH 03/13] [ticket/14742] Small fixes to migrator PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 3e69d9613e..8637f71414 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -87,7 +87,7 @@ class migrator * * @var migrator_output_handler_interface */ - public $output_handler; + protected $output_handler; /** * Constructor of the database migrator @@ -333,7 +333,7 @@ class migrator $state['migration_data_done'] = ($result === true); $state['migration_end_time'] = ($result === true) ? time() : 0; - if ($state['migration_schema_done']) + if ($state['migration_data_done']) { $this->output_handler->write(array('MIGRATION_DATA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); } @@ -347,7 +347,6 @@ class migrator // Revert the schema changes $this->revert_do($name); - // Rethrow exception throw $e; } } @@ -518,7 +517,6 @@ class migrator $result = $this->run_step($reverse_step, false, !$revert); } - // rethrow the exception throw $e; } } From 2ee8bd0c4a0379961c7ec00376c216822672057f Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Wed, 10 Aug 2016 17:57:29 +0200 Subject: [PATCH 04/13] [ticket/14742] Add test for reverse_update_data() PHPBB3-14742 --- tests/migrator/reverse_update_data_test.php | 56 +++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/migrator/reverse_update_data_test.php diff --git a/tests/migrator/reverse_update_data_test.php b/tests/migrator/reverse_update_data_test.php new file mode 100644 index 0000000000..b85e48c64c --- /dev/null +++ b/tests/migrator/reverse_update_data_test.php @@ -0,0 +1,56 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class reverse_update_data_test extends phpbb_test_case +{ + /** @var \phpbb\db\migration\helper */ + protected $helper; + + public function setUp() + { + parent::setUp(); + + $this->helper = new \phpbb\db\migration\helper(); + } + + public function update_data_provider() + { + return array( + array( + array( + array('config.add', array('foobar', 1)), + array('if', array( + (false === true), + array('permission.add', array('some_data')), + )), + array('config.remove', array('foobar')), + array('custom', array(array($this, 'foo_bar'))), + array('tool.method', array('test_data')), + ), + array( + array('tool.reverse', array('method', 'test_data')), + array('config.reverse', array('remove', 'foobar')), + array('config.reverse', array('add', 'foobar', 1)), + ), + ), + ); + } + + /** + * @dataProvider update_data_provider + */ + public function test_get_schema_steps($data_changes, $expected) + { + $this->assertEquals($expected, $this->helper->reverse_update_data($data_changes)); + } +} From 3346609126893799169dc678da732d99d506fb9f Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 01:25:45 +0200 Subject: [PATCH 05/13] [ticket/14742] Display message if reverting schema is in progress PHPBB3-14742 --- phpBB/language/en/migrator.php | 1 + phpBB/phpbb/db/migrator.php | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php index e172b2b327..78364319a1 100644 --- a/phpBB/language/en/migrator.php +++ b/phpBB/language/en/migrator.php @@ -58,6 +58,7 @@ $lang = array_merge($lang, array( 'MIGRATION_REVERT_DATA_IN_PROGRESS' => 'Reverting Data: %1$s; Time: %2$.2f seconds', 'MIGRATION_REVERT_DATA_RUNNING' => 'Reverting Data: %s.', 'MIGRATION_REVERT_SCHEMA_DONE' => 'Reverted Schema: %1$s; Time: %2$.2f seconds', + 'MIGRATION_REVERT_SCHEMA_IN_PROGRESS' => 'Reverting Schema: %1$s; Time: %2$.2f seconds', 'MIGRATION_REVERT_SCHEMA_RUNNING' => 'Reverting Schema: %s.', 'MIGRATION_INVALID_DATA_MISSING_CONDITION' => 'A migration is invalid. An if statement helper is missing a condition.', diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 000859f418..030afe07ad 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -454,7 +454,7 @@ class migrator $this->set_migration_state($name, $state); $elapsed_time = microtime(true) - $elapsed_time; - if ($state['migration_data_done']) + if (!$state['migration_data_done']) { $this->output_handler->write(array('MIGRATION_REVERT_DATA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); } @@ -474,6 +474,7 @@ class migrator $state['migration_data_state'] = ($result === true) ? '' : $result; $state['migration_schema_done'] = ($result === true) ? false : true; + $elapsed_time = microtime(true) - $elapsed_time; if (!$state['migration_schema_done']) { $sql = 'DELETE FROM ' . $this->migrations_table . " @@ -481,10 +482,13 @@ class migrator $this->db->sql_query($sql); unset($this->migration_state[$name]); - } - $elapsed_time = microtime(true) - $elapsed_time; - $this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + $this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + } + else + { + $this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_IN_PROGRESS', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_VERY_VERBOSE); + } } return true; From 6078bae7f8888f6c1d8bdc9a409b3e0a9cb3cc49 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 13:31:23 +0200 Subject: [PATCH 06/13] [ticket/14742] Fix schema update First make it work, then avoid a loop PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 8637f71414..aaa7769cee 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -470,6 +470,9 @@ class migrator $steps = array_reverse($steps); } + end($steps); + $last_step_identifier = key($steps); + foreach ($steps as $step_identifier => $step) { $last_result = 0; @@ -486,6 +489,12 @@ class migrator // Set state to false since we reached the point we were at $state = false; + + // There is a programmed tendency to get stuck in this case + if (strpos($step[0], 'dbtools') === 0 && ($last_result === null || $last_result === true)) + { + continue; + } } try @@ -494,7 +503,8 @@ class migrator // After any schema update step we allow to pause, since // database changes can take quite some time $result = $this->run_step($step, $last_result, $revert); - if ($result !== null && $result !== true && strpos($step[0], 'dbtools') !== 0) + if (($result !== null && $result !== true) || + (strpos($step[0], 'dbtools') === 0 && $step_identifier !== $last_step_identifier)) { return serialize(array( 'result' => $result, From b00a39b9adeca86f2c2777a74d6f4f8325ee7743 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 15:37:53 +0200 Subject: [PATCH 07/13] [ticket/14742] Make $last_run_migration protected PHPBB3-14742 --- phpBB/install/database_update.php | 7 ------- phpBB/phpbb/db/migrator.php | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 0ea6eeffd7..f367ae1fc0 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -211,13 +211,6 @@ while (!$migrator->finished()) phpbb_end_update($cache, $config); } - $state = array_merge(array( - 'migration_schema_done' => false, - 'migration_data_done' => false, - ), - $migrator->last_run_migration['state'] - ); - // Are we approaching the time limit? If so we want to pause the update and continue after refreshing if ((time() - $update_start_time) >= $safe_time_limit) { diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index aaa7769cee..965d117b0b 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -80,7 +80,7 @@ class migrator * * @var array */ - public $last_run_migration = false; + protected $last_run_migration = false; /** * The output handler. A null handler is configured by default. @@ -160,6 +160,19 @@ class migrator $this->db->sql_return_on_error(false); } + /** + * Get an array with information about the last migration run. + * + * The array contains 'name', 'class' and 'state'. 'effectively_installed' is set + * and set to true if the last migration was effectively_installed. + * + * @return array + */ + public function get_last_run_migration() + { + return $this->last_run_migration; + } + /** * Sets the list of available migration class names to the given array. * From 52afa74f4e6080952a72b6f40f15e6ce0a547b49 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 16:38:56 +0200 Subject: [PATCH 08/13] [ticket/14742] Avoid loop while reverting schema PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 965d117b0b..b82b1b918a 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -457,8 +457,13 @@ class migrator WHERE migration_name = '" . $this->db->sql_escape($name) . "'"; $this->db->sql_query($sql); + $this->last_run_migration = false; unset($this->migration_state[$name]); } + else + { + $this->set_migration_state($name, $state); + } } return true; @@ -503,8 +508,8 @@ class migrator // Set state to false since we reached the point we were at $state = false; - // There is a programmed tendency to get stuck in this case - if (strpos($step[0], 'dbtools') === 0 && ($last_result === null || $last_result === true)) + // There is a tendency to get stuck in some cases + if ($last_result === null || $last_result === true) { continue; } From c12d67cd900514f9752d7bb73928870dbab0a0ce Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 18:11:39 +0200 Subject: [PATCH 09/13] [ticket/14742] Add test for (not) reverting if PHPBB3-14742 --- tests/dbal/migration/if.php | 4 ++-- tests/dbal/migrator_test.php | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/dbal/migration/if.php b/tests/dbal/migration/if.php index 98a66526ed..481250ea77 100644 --- a/tests/dbal/migration/if.php +++ b/tests/dbal/migration/if.php @@ -36,13 +36,13 @@ class phpbb_dbal_migration_if extends \phpbb\db\migration\migration { global $migrator_test_if_true_failed; - $migrator_test_if_true_failed = false; + $migrator_test_if_true_failed = !$migrator_test_if_true_failed; } function test_false() { global $migrator_test_if_false_failed; - $migrator_test_if_false_failed = true; + $migrator_test_if_false_failed = !$migrator_test_if_false_failed; } } diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php index 4c4306888c..798200eef1 100644 --- a/tests/dbal/migrator_test.php +++ b/tests/dbal/migrator_test.php @@ -156,6 +156,14 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case $this->assertFalse($migrator_test_if_true_failed, 'True test failed'); $this->assertFalse($migrator_test_if_false_failed, 'False test failed'); + + while ($this->migrator->migration_state('phpbb_dbal_migration_if') !== false) + { + $this->migrator->revert('phpbb_dbal_migration_if'); + } + + $this->assertFalse($migrator_test_if_true_failed, 'True test after revert failed'); + $this->assertFalse($migrator_test_if_false_failed, 'False test after revert failed'); } public function test_recall() From 03be89ebd7bfd95e8586d0d13076a90afec243ee Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 23:28:54 +0200 Subject: [PATCH 10/13] [ticket/14742] Fix progress bar in database updater Because of the new way, schema update steps are handled, the already misleading progress bar was even more misleading. This should fix it. PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 22 +++++++++++++++++++ .../module/update_database/task/update.php | 19 +++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 89f05f7b40..9e76668655 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -204,6 +204,28 @@ class migrator return $this->migrations; } + /** + * Get the list of available and not installed migration class names + * + * @return array + */ + public function get_installable_migrations() + { + $unfinished_migrations = array(); + + foreach ($this->migrations as $name) + { + if (!isset($this->migration_state[$name]) || + !$this->migration_state[$name]['migration_schema_done'] || + !$this->migration_state[$name]['migration_data_done']) + { + $unfinished_migrations[] = $name; + } + } + + return $unfinished_migrations; + } + /** * Runs a single update step from the next migration to be applied. * diff --git a/phpBB/phpbb/install/module/update_database/task/update.php b/phpBB/phpbb/install/module/update_database/task/update.php index 9fed2317e9..d167181125 100644 --- a/phpBB/phpbb/install/module/update_database/task/update.php +++ b/phpBB/phpbb/install/module/update_database/task/update.php @@ -140,7 +140,14 @@ class update extends task_base ->get_classes(); $this->migrator->set_migrations($migrations); - $migration_count = count($this->migrator->get_migrations()); + + $migration_count = $this->installer_config->get('database_update_migrations', -1); + if ($migration_count < 0) + { + $migration_count = count($this->migrator->get_installable_migrations()); + $this->installer_config->set('database_update_migrations', $migration_count); + } + $this->iohandler->set_task_count($migration_count, true); $this->installer_config->set_task_progress_count($migration_count); $progress_count = $this->installer_config->get('database_update_count', 0); @@ -150,8 +157,14 @@ class update extends task_base try { $this->migrator->update(); - $progress_count++; - $this->iohandler->set_progress('STAGE_UPDATE_DATABASE', $progress_count); + + $last_run_migration = $this->migrator->get_last_run_migration(); + + if ($last_run_migration['state']['migration_data_done']) + { + $progress_count++; + $this->iohandler->set_progress('STAGE_UPDATE_DATABASE', $progress_count); + } } catch (exception $e) { From 4a92a8efb549c740f61655836f85de9be5c5b548 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Fri, 12 Aug 2016 01:01:14 +0200 Subject: [PATCH 11/13] [ticket/14742] Improve verbosity of migrator output PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index b82b1b918a..eb3bc7eaf5 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -310,7 +310,9 @@ class migrator if (!$state['migration_schema_done']) { - $this->output_handler->write(array('MIGRATION_SCHEMA_RUNNING', $name), migrator_output_handler_interface::VERBOSITY_VERBOSE); + $verbosity = empty($state['migration_data_state']) ? + migrator_output_handler_interface::VERBOSITY_VERBOSE : migrator_output_handler_interface::VERBOSITY_DEBUG; + $this->output_handler->write(array('MIGRATION_SCHEMA_RUNNING', $name), $verbosity); $this->last_run_migration['task'] = 'process_schema_step'; $elapsed_time = microtime(true); @@ -334,7 +336,9 @@ class migrator { try { - $this->output_handler->write(array('MIGRATION_DATA_RUNNING', $name), migrator_output_handler_interface::VERBOSITY_VERBOSE); + $verbosity = empty($state['migration_data_state']) ? + migrator_output_handler_interface::VERBOSITY_VERBOSE : migrator_output_handler_interface::VERBOSITY_DEBUG; + $this->output_handler->write(array('MIGRATION_DATA_RUNNING', $name), $verbosity); $this->last_run_migration['task'] = 'process_data_step'; From 263fbe54fcf03fe55f738146d7c6359108a4b48f Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Fri, 12 Aug 2016 02:03:52 +0200 Subject: [PATCH 12/13] [ticket/14742] Enhance measured time for migrations PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index eb3bc7eaf5..65db4b74bf 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -152,6 +152,7 @@ class migrator $this->migration_state[$migration['migration_name']] = $migration; $this->migration_state[$migration['migration_name']]['migration_depends_on'] = unserialize($migration['migration_depends_on']); + $this->migration_state[$migration['migration_name']]['migration_data_state'] = !empty($migration['migration_data_state']) ? unserialize($migration['migration_data_state']) : ''; } } @@ -315,17 +316,26 @@ class migrator $this->output_handler->write(array('MIGRATION_SCHEMA_RUNNING', $name), $verbosity); $this->last_run_migration['task'] = 'process_schema_step'; + + $total_time = (is_array($state['migration_data_state']) && isset($state['migration_data_state']['_total_time'])) ? + $state['migration_data_state']['_total_time'] : 0.0; $elapsed_time = microtime(true); $steps = $this->helper->get_schema_steps($migration->update_schema()); $result = $this->process_data_step($steps, $state['migration_data_state']); $elapsed_time = microtime(true) - $elapsed_time; + $total_time += $elapsed_time; + + if (is_array($result)) + { + $result['_total_time'] = $total_time; + } $state['migration_data_state'] = ($result === true) ? '' : $result; $state['migration_schema_done'] = ($result === true); if ($state['migration_schema_done']) { - $this->output_handler->write(array('MIGRATION_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + $this->output_handler->write(array('MIGRATION_SCHEMA_DONE', $name, $total_time), migrator_output_handler_interface::VERBOSITY_NORMAL); } else { @@ -342,9 +352,17 @@ class migrator $this->last_run_migration['task'] = 'process_data_step'; + $total_time = (is_array($state['migration_data_state']) && isset($state['migration_data_state']['_total_time'])) ? + $state['migration_data_state']['_total_time'] : 0.0; $elapsed_time = microtime(true); $result = $this->process_data_step($migration->update_data(), $state['migration_data_state']); $elapsed_time = microtime(true) - $elapsed_time; + $total_time += $elapsed_time; + + if (is_array($result)) + { + $result['_total_time'] = $total_time; + } $state['migration_data_state'] = ($result === true) ? '' : $result; $state['migration_data_done'] = ($result === true); @@ -352,7 +370,7 @@ class migrator if ($state['migration_data_done']) { - $this->output_handler->write(array('MIGRATION_DATA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + $this->output_handler->write(array('MIGRATION_DATA_DONE', $name, $total_time), migrator_output_handler_interface::VERBOSITY_NORMAL); } else { @@ -484,7 +502,7 @@ class migrator */ protected function process_data_step($steps, $state, $revert = false) { - $state = ($state) ? unserialize($state) : false; + $state = is_array($state) ? $state : false; // reverse order of steps if reverting if ($revert === true) @@ -528,10 +546,10 @@ class migrator if (($result !== null && $result !== true) || (strpos($step[0], 'dbtools') === 0 && $step_identifier !== $last_step_identifier)) { - return serialize(array( + return array( 'result' => $result, 'step' => $step_identifier, - )); + ); } } catch (\phpbb\db\migration\exception $e) @@ -702,6 +720,7 @@ class migrator { $migration_row = $state; $migration_row['migration_depends_on'] = serialize($state['migration_depends_on']); + $migration_row['migration_data_state'] = !empty($state['migration_data_state']) ? serialize($state['migration_data_state']) : ''; if (isset($this->migration_state[$name])) { From 758fe20f4be7178fd4b9fd6ce48c5342cfcbce27 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Fri, 12 Aug 2016 03:45:56 +0200 Subject: [PATCH 13/13] [ticket/14742] Further improve progress bar in db updater PHPBB3-14742 --- .../module/update_database/task/update.php | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/phpBB/phpbb/install/module/update_database/task/update.php b/phpBB/phpbb/install/module/update_database/task/update.php index d167181125..c69dafaa10 100644 --- a/phpBB/phpbb/install/module/update_database/task/update.php +++ b/phpBB/phpbb/install/module/update_database/task/update.php @@ -141,16 +141,17 @@ class update extends task_base $this->migrator->set_migrations($migrations); - $migration_count = $this->installer_config->get('database_update_migrations', -1); - if ($migration_count < 0) + $migration_step_count = $this->installer_config->get('database_update_migration_steps', -1); + if ($migration_step_count < 0) { - $migration_count = count($this->migrator->get_installable_migrations()); - $this->installer_config->set('database_update_migrations', $migration_count); + $migration_step_count = count($this->migrator->get_installable_migrations()) * 2; + $this->installer_config->set('database_update_migration_steps', $migration_step_count); } - $this->iohandler->set_task_count($migration_count, true); - $this->installer_config->set_task_progress_count($migration_count); $progress_count = $this->installer_config->get('database_update_count', 0); + $restart_progress_bar = ($progress_count === 0); // Only "restart" when the update runs for the first time + $this->iohandler->set_task_count($migration_step_count, $restart_progress_bar); + $this->installer_config->set_task_progress_count($migration_step_count); while (!$this->migrator->finished()) { @@ -159,12 +160,17 @@ class update extends task_base $this->migrator->update(); $last_run_migration = $this->migrator->get_last_run_migration(); - - if ($last_run_migration['state']['migration_data_done']) + if (isset($last_run_migration['effectively_installed']) && $last_run_migration['effectively_installed']) + { + $progress_count += 2; + } + else if (($last_run_migration['task'] === 'process_schema_step' && $last_run_migration['state']['migration_schema_done']) || + ($last_run_migration['task'] === 'process_data_step' && $last_run_migration['state']['migration_data_done'])) { $progress_count++; - $this->iohandler->set_progress('STAGE_UPDATE_DATABASE', $progress_count); } + + $this->iohandler->set_progress('STAGE_UPDATE_DATABASE', $progress_count); } catch (exception $e) {