From e555a4439f5c4024b4287d4bd2be969b90c08890 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Wed, 11 Sep 2013 14:27:52 +0100 Subject: [PATCH] MDL-41722 Backup: Very large course times out on user interface pages Adds 'Preparing page display' progress bars for user interface pages if they take a long time to display. Also adds changes where other parts of the backup progress timed out on long backups. After this change, and MDL-41838, it is finally possible on my dev server to successfully back up the 'XL' test course. --- backup/backup.php | 34 ++++++++++-- backup/moodle2/backup_stepslib.php | 6 ++- .../dbops/backup_controller_dbops.class.php | 41 +++++++++++++-- backup/util/helper/backup_helper.class.php | 9 +++- backup/util/includes/backup_includes.php | 1 + backup/util/ui/backup_ui_stage.class.php | 52 ++++++++++++++++++- 6 files changed, 129 insertions(+), 14 deletions(-) diff --git a/backup/backup.php b/backup/backup.php index cf7d3bee59b..aa1ad4d0e85 100644 --- a/backup/backup.php +++ b/backup/backup.php @@ -88,14 +88,30 @@ if (!($bc = backup_ui::load_controller($backupid))) { backup::INTERACTIVE_YES, backup::MODE_GENERAL, $USER->id); } $backup = new backup_ui($bc); -$backup->process(); -$PAGE->set_title($heading.': '.$backup->get_stage_name()); +$PAGE->set_title($heading); $PAGE->set_heading($heading); -$PAGE->navbar->add($backup->get_stage_name()); $renderer = $PAGE->get_renderer('core','backup'); echo $OUTPUT->header(); + +// Prepare a progress bar which can display optionally during long-running +// operations while setting up the UI. +$slowprogress = new core_backup_display_progress_if_slow(get_string('preparingui', 'backup')); + +$previous = optional_param('previous', false, PARAM_BOOL); +if ($backup->get_stage() == backup_ui::STAGE_SCHEMA && !$previous) { + // After schema stage, we are probably going to get to the confirmation stage, + // The confirmation stage has 2 sets of progress, so this is needed to prevent + // it showing 2 progress bars. + $twobars = true; + $slowprogress->start_progress('', 2); +} else { + $twobars = false; +} +$backup->get_controller()->set_progress($slowprogress); +$backup->process(); + if ($backup->enforce_changed_dependencies()) { debugging('Your settings have been altered due to unmet dependencies', DEBUG_DEVELOPER); } @@ -112,8 +128,16 @@ if ($backup->get_stage() == backup_ui::STAGE_FINAL) { $backup->save_controller(); } +// Displaying UI can require progress reporting, so do it here before outputting +// the backup stage bar (as part of the existing progress bar, if required). +$ui = $backup->display($renderer); +if ($twobars) { + $slowprogress->end_progress(); +} + echo $renderer->progress_bar($backup->get_progress_bar()); -echo $backup->display($renderer); + +echo $ui; $backup->destroy(); unset($backup); -echo $OUTPUT->footer(); \ No newline at end of file +echo $OUTPUT->footer(); diff --git a/backup/moodle2/backup_stepslib.php b/backup/moodle2/backup_stepslib.php index 922318c349b..fa22a79fc4d 100644 --- a/backup/moodle2/backup_stepslib.php +++ b/backup/moodle2/backup_stepslib.php @@ -1589,7 +1589,8 @@ class backup_main_structure_step extends backup_structure_step { $info['original_system_contextid'] = context_system::instance()->id; // Get more information from controller - list($dinfo, $cinfo, $sinfo) = backup_controller_dbops::get_moodle_backup_information($this->get_backupid()); + list($dinfo, $cinfo, $sinfo) = backup_controller_dbops::get_moodle_backup_information( + $this->get_backupid(), $this->get_task()->get_progress()); // Define elements @@ -1753,7 +1754,8 @@ class backup_store_backup_file extends backup_execution_step { $has_file_references = backup_controller_dbops::backup_includes_file_references($this->get_backupid()); // Perform storage and return it (TODO: shouldn't be array but proper result object) return array( - 'backup_destination' => backup_helper::store_backup_file($this->get_backupid(), $zipfile), + 'backup_destination' => backup_helper::store_backup_file($this->get_backupid(), $zipfile, + $this->task->get_progress()), 'include_file_references_to_external_content' => $has_file_references ); } diff --git a/backup/util/dbops/backup_controller_dbops.class.php b/backup/util/dbops/backup_controller_dbops.class.php index 9fd95acf990..cb4577c80dc 100644 --- a/backup/util/dbops/backup_controller_dbops.class.php +++ b/backup/util/dbops/backup_controller_dbops.class.php @@ -350,15 +350,32 @@ abstract class backup_controller_dbops extends backup_dbops { /** * Get details information for main moodle_backup.xml file, extracting it from - * the specified controller + * the specified controller. + * + * If you specify the progress monitor, this will start a new progress section + * to track progress in processing (in case this task takes a long time). + * + * @param string $backupid Backup ID + * @param core_backup_progress $progress Optional progress monitor */ - public static function get_moodle_backup_information($backupid) { + public static function get_moodle_backup_information($backupid, + core_backup_progress $progress = null) { + + // Start tracking progress if required (for load_controller). + if ($progress) { + $progress->start_progress('get_moodle_backup_information', 2); + } $detailsinfo = array(); // Information details $contentsinfo= array(); // Information about backup contents $settingsinfo= array(); // Information about backup settings $bc = self::load_controller($backupid); // Load controller + // Note that we have loaded controller. + if ($progress) { + $progress->progress(1); + } + // Details info $detailsinfo['id'] = $bc->get_id(); $detailsinfo['backup_id'] = $bc->get_backupid(); @@ -377,8 +394,15 @@ abstract class backup_controller_dbops extends backup_dbops { $contentsinfo['sections'] = array(); $contentsinfo['course'] = array(); + // Get tasks and start nested progress. + $tasks = $bc->get_plan()->get_tasks(); + if ($progress) { + $progress->start_progress('get_moodle_backup_information', count($tasks)); + $done = 1; + } + // Contents info (extract information from tasks) - foreach ($bc->get_plan()->get_tasks() as $task) { + foreach ($tasks as $task) { if ($task instanceof backup_activity_task) { // Activity task @@ -407,10 +431,21 @@ abstract class backup_controller_dbops extends backup_dbops { list($contentinfo, $settings) = self::get_root_backup_information($task); $settingsinfo = array_merge($settingsinfo, $settings); } + + // Report task handled. + if ($progress) { + $progress->progress($done++); + } } $bc->destroy(); // Always need to destroy controller to handle circular references + // Finish progress reporting. + if ($progress) { + $progress->end_progress(); + $progress->end_progress(); + } + return array(array((object)$detailsinfo), $contentsinfo, $settingsinfo); } diff --git a/backup/util/helper/backup_helper.class.php b/backup/util/helper/backup_helper.class.php index 87b6ebaa968..e1c9d1e9ae5 100644 --- a/backup/util/helper/backup_helper.class.php +++ b/backup/util/helper/backup_helper.class.php @@ -179,17 +179,22 @@ abstract class backup_helper { * * Note: the $filepath is deleted if the backup file is created successfully * + * If you specify the progress monitor, this will start a new progress section + * to track progress in processing (in case this task takes a long time). + * * @param int $backupid * @param string $filepath zip file containing the backup + * @param core_backup_progress $progress Optional progress monitor * @return stored_file if created, null otherwise * * @throws moodle_exception in case of any problems */ - static public function store_backup_file($backupid, $filepath) { + static public function store_backup_file($backupid, $filepath, core_backup_progress $progress = null) { global $CFG; // First of all, get some information from the backup_controller to help us decide - list($dinfo, $cinfo, $sinfo) = backup_controller_dbops::get_moodle_backup_information($backupid); + list($dinfo, $cinfo, $sinfo) = backup_controller_dbops::get_moodle_backup_information( + $backupid, $progress); // Extract useful information to decide $hasusers = (bool)$sinfo['users']->value; // Backup has users diff --git a/backup/util/includes/backup_includes.php b/backup/util/includes/backup_includes.php index 2a4f06bedcc..5b19a2b388a 100644 --- a/backup/util/includes/backup_includes.php +++ b/backup/util/includes/backup_includes.php @@ -74,6 +74,7 @@ require_once($CFG->dirroot . '/backup/util/loggers/output_indented_logger.class. require_once($CFG->dirroot . '/backup/util/progress/core_backup_progress.class.php'); require_once($CFG->dirroot . '/backup/util/progress/core_backup_null_progress.class.php'); require_once($CFG->dirroot . '/backup/util/progress/core_backup_display_progress.class.php'); +require_once($CFG->dirroot . '/backup/util/progress/core_backup_display_progress_if_slow.class.php'); require_once($CFG->dirroot . '/backup/util/settings/setting_dependency.class.php'); require_once($CFG->dirroot . '/backup/util/settings/base_setting.class.php'); require_once($CFG->dirroot . '/backup/util/settings/backup_setting.class.php'); diff --git a/backup/util/ui/backup_ui_stage.class.php b/backup/util/ui/backup_ui_stage.class.php index 4065212648c..257a2ec10d6 100644 --- a/backup/util/ui/backup_ui_stage.class.php +++ b/backup/util/ui/backup_ui_stage.class.php @@ -170,6 +170,11 @@ class backup_ui_stage_initial extends backup_ui_stage { * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class backup_ui_stage_schema extends backup_ui_stage { + /** + * @var int Maximum number of settings to add to form at once + */ + const MAX_SETTINGS_BATCH = 1000; + /** * Schema stage constructor * @param backup_moodleform $ui @@ -236,6 +241,14 @@ class backup_ui_stage_schema extends backup_ui_stage { $courseheading = false; $add_settings = array(); $dependencies = array(); + + // Track progress through each stage. + $progress = $this->ui->get_controller()->get_progress(); + $progress->start_progress('Initialise stage form', 3); + + // Get settings for all tasks. + $progress->start_progress('', count($tasks)); + $done = 1; foreach ($tasks as $task) { if (!($task instanceof backup_root_task)) { if (!$courseheading) { @@ -263,11 +276,37 @@ class backup_ui_stage_schema extends backup_ui_stage { } } } + // Update progress. + $progress->progress($done++); } - $form->add_settings($add_settings); + $progress->end_progress(); + + // Add settings for tasks in batches of up to 1000. Adding settings + // in larger batches improves performance, but if it takes too long, + // we won't be able to update the progress bar so the backup might + // time out. 1000 is chosen to balance this. + $numsettings = count($add_settings); + $progress->start_progress('', ceil($numsettings / self::MAX_SETTINGS_BATCH)); + $start = 0; + $done = 1; + while($start < $numsettings) { + $length = min(self::MAX_SETTINGS_BATCH, $numsettings - $start); + $form->add_settings(array_slice($add_settings, $start, $length)); + $start += $length; + $progress->progress($done++); + } + $progress->end_progress(); + + $progress->start_progress('', count($dependencies)); + $done = 1; foreach ($dependencies as $depsetting) { $form->add_dependencies($depsetting); + $progress->progress($done++); } + $progress->end_progress(); + + // End overall progress through creating form. + $progress->end_progress(); $this->stageform = $form; } return $this->stageform; @@ -357,7 +396,13 @@ class backup_ui_stage_confirmation extends backup_ui_stage { } } - foreach ($this->ui->get_tasks() as $task) { + // Track progress through tasks. + $progress = $this->ui->get_controller()->get_progress(); + $tasks = $this->ui->get_tasks(); + $progress->start_progress('initialise_stage_form', count($tasks)); + $done = 1; + + foreach ($tasks as $task) { if ($task instanceof backup_root_task) { // If its a backup root add a root settings heading to group nicely $form->add_heading('rootsettings', get_string('rootsettings', 'backup')); @@ -373,7 +418,10 @@ class backup_ui_stage_confirmation extends backup_ui_stage { $form->add_fixed_setting($setting, $task); } } + // Update progress. + $progress->progress($done++); } + $progress->end_progress(); $this->stageform = $form; } return $this->stageform;