MDL-41722 Backup: Very large course times out on user interface pages
authorsam marshall <s.marshall@open.ac.uk>
Wed, 11 Sep 2013 13:27:52 +0000 (14:27 +0100)
committersam marshall <s.marshall@open.ac.uk>
Fri, 4 Oct 2013 09:32:13 +0000 (10:32 +0100)
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
backup/moodle2/backup_stepslib.php
backup/util/dbops/backup_controller_dbops.class.php
backup/util/helper/backup_helper.class.php
backup/util/includes/backup_includes.php
backup/util/ui/backup_ui_stage.class.php

index cf7d3be..aa1ad4d 100644 (file)
@@ -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();
index 922318c..fa22a79 100644 (file)
@@ -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
         );
     }
index 9fd95ac..cb4577c 100644 (file)
@@ -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);
     }
 
index 87b6eba..e1c9d1e 100644 (file)
@@ -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
index 2a4f06b..5b19a2b 100644 (file)
@@ -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');
index 4065212..257a2ec 100644 (file)
@@ -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;