MDL-60981 core_search: New indexpriority field in search index queue

Adds indexpriority field to the database table which holds a queue of
indexing requests. This allows for potentially large area reindexes
to have a lower priority, so as not to halt the special indexes that
run after a course restore.

lib/db/install.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="lib/db" VERSION="20171205" COMMENT="XMLDB file for core Moodle tables"
+<XMLDB PATH="lib/db" VERSION="20171222" COMMENT="XMLDB file for core Moodle tables"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
 >
@@ -3770,11 +3770,15 @@
         <FIELD NAME="timerequested" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Time at which this index update was requested."/>
         <FIELD NAME="partialarea" TYPE="char" LENGTH="255" NOTNULL="true" SEQUENCE="false" COMMENT="If processing of this context partially completed, set to the area that needs processing next. Blank indicates not processed yet."/>
         <FIELD NAME="partialtime" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="If processing partially completed, set to the timestamp within the next area where processing should start. 0 indicates not processed yet."/>
+        <FIELD NAME="indexpriority" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Priority value so that important requests can be dealt with first; higher numbers are processed first"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
         <KEY NAME="contextid" TYPE="foreign" FIELDS="contextid" REFTABLE="context" REFFIELDS="id"/>
       </KEYS>
+      <INDEXES>
+        <INDEX NAME="indexprioritytimerequested" UNIQUE="false" FIELDS="indexpriority, timerequested"/>
+      </INDEXES>
     </TABLE>
   </TABLES>
 </XMLDB>

lib/db/upgrade.php

@@ -1902,5 +1902,38 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2017121900.00);
     }
 
+    if ($oldversion < 2017122200.01) {
+
+        // Define field indexpriority to be added to search_index_requests. Allow null initially.
+        $table = new xmldb_table('search_index_requests');
+        $field = new xmldb_field('indexpriority', XMLDB_TYPE_INTEGER, '10',
+                null, null, null, null, 'partialtime');
+
+        // Conditionally add field.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+
+            // Set existing values to 'normal' value (100).
+            $DB->set_field('search_index_requests', 'indexpriority', 100);
+
+            // Now make the field 'NOT NULL'.
+            $field = new xmldb_field('indexpriority', XMLDB_TYPE_INTEGER, '10',
+                    null, XMLDB_NOTNULL, null, null, 'partialtime');
+            $dbman->change_field_notnull($table, $field);
+        }
+
+        // Define index indexprioritytimerequested (not unique) to be added to search_index_requests.
+        $index = new xmldb_index('indexprioritytimerequested', XMLDB_INDEX_NOTUNIQUE,
+                array('indexpriority', 'timerequested'));
+
+        // Conditionally launch add index indexprioritytimerequested.
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2017122200.01);
+    }
+
     return true;
 }

search/classes/manager.php

@@ -87,6 +87,16 @@ class manager {
      */
     const DISPLAY_INDEXING_PROGRESS_EVERY = 30.0;
 
+    /**
+     * @var int Context indexing: normal priority.
+     */
+    const INDEX_PRIORITY_NORMAL = 100;
+
+    /**
+     * @var int Context indexing: low priority for reindexing.
+     */
+    const INDEX_PRIORITY_REINDEXING = 50;
+
     /**
      * @var \core_search\base[] Enabled search areas.
      */
@@ -1147,32 +1157,50 @@ class manager {
      *
      * @param \context $context Context to index within
      * @param string $areaid Area to index, '' = all areas
+     * @param int $priority Priority (INDEX_PRIORITY_xx constant)
      */
-    public static function request_index(\context $context, $areaid = '') {
+    public static function request_index(\context $context, $areaid = '',
+            $priority = self::INDEX_PRIORITY_NORMAL) {
         global $DB;
 
         // Check through existing requests for this context or any parent context.
         list ($contextsql, $contextparams) = $DB->get_in_or_equal(
                 $context->get_parent_context_ids(true));
         $existing = $DB->get_records_select('search_index_requests',
-                'contextid ' . $contextsql, $contextparams, '', 'id, searcharea, partialarea');
+                'contextid ' . $contextsql, $contextparams, '',
+                'id, searcharea, partialarea, indexpriority');
         foreach ($existing as $rec) {
             // If we haven't started processing the existing request yet, and it covers the same
             // area (or all areas) then that will be sufficient so don't add anything else.
             if ($rec->partialarea === '' && ($rec->searcharea === $areaid || $rec->searcharea === '')) {
-                return;
+                // If the existing request has the same (or higher) priority, no need to add anything.
+                if ($rec->indexpriority >= $priority) {
+                    return;
+                }
+                // The existing request has lower priority. If it is exactly the same, then just
+                // adjust the priority of the existing request.
+                if ($rec->searcharea === $areaid) {
+                    $DB->set_field('search_index_requests', 'indexpriority', $priority,
+                            ['id' => $rec->id]);
+                    return;
+                }
+                // The existing request would cover this area but is a lower priority. We need to
+                // add the new request even though that means we will index part of it twice.
             }
         }
 
         // No suitable existing request, so add a new one.
         $newrecord = [ 'contextid' => $context->id, 'searcharea' => $areaid,
-                'timerequested' => time(), 'partialarea' => '', 'partialtime' => 0 ];
+                'timerequested' => (int)self::get_current_time(),
+                'partialarea' => '', 'partialtime' => 0,
+                'indexpriority' => $priority ];
         $DB->insert_record('search_index_requests', $newrecord);
     }
 
     /**
-     * Processes outstanding index requests. This will take the first item from the queue and
-     * process it, continuing until an optional time limit is reached.
+     * Processes outstanding index requests. This will take the first item from the queue (taking
+     * account the indexing priority) and process it, continuing until an optional time limit is
+     * reached.
      *
      * If there are no index requests, the function will do nothing.
      *
@@ -1186,7 +1214,6 @@ class manager {
             $progress = new \null_progress_trace();
         }
 
-        $complete = false;
         $before = self::get_current_time();
         if ($timelimit) {
             $stopat = $before + $timelimit;
@@ -1194,11 +1221,10 @@ class manager {
         while (true) {
             // Retrieve first request, using fully defined ordering.
             $requests = $DB->get_records('search_index_requests', null,
-                    'timerequested, contextid, searcharea',
+                    'indexpriority DESC, timerequested, contextid, searcharea',
                     'id, contextid, searcharea, partialarea, partialtime', 0, 1);
             if (!$requests) {
                 // If there are no more requests, stop.
-                $complete = true;
                 break;
             }
             $request = reset($requests);
@@ -1208,6 +1234,12 @@ class manager {
             $beforeindex = self::get_current_time();
             if ($timelimit) {
                 $remainingtime = $stopat - $beforeindex;
+
+                // If the time limit expired already, stop now. (Otherwise we might accidentally
+                // index with no time limit or a negative time limit.)
+                if ($remainingtime <= 0) {
+                    break;
+                }
             }
 
             // Show a message before each request, indicating what will be indexed.

search/tests/manager_test.php

@@ -941,6 +941,39 @@ class search_manager_testcase extends advanced_testcase {
         // if we had already begun processing the previous entry.)
         \core_search\manager::request_index($forum2ctx);
         $this->assertEquals(4, $DB->count_records('search_index_requests'));
+
+        // Clear queue and do tests relating to priority.
+        $DB->delete_records('search_index_requests');
+
+        // Request forum 1, specific area, priority 100.
+        \core_search\manager::request_index($forum1ctx, 'forum-post', 100);
+        $results = array_values($DB->get_records('search_index_requests', null, 'id'));
+        $this->assertCount(1, $results);
+        $this->assertEquals(100, $results[0]->indexpriority);
+
+        // Request forum 1, same area, lower priority; no change.
+        \core_search\manager::request_index($forum1ctx, 'forum-post', 99);
+        $results = array_values($DB->get_records('search_index_requests', null, 'id'));
+        $this->assertCount(1, $results);
+        $this->assertEquals(100, $results[0]->indexpriority);
+
+        // Request forum 1, same area, higher priority; priority stored changes.
+        \core_search\manager::request_index($forum1ctx, 'forum-post', 101);
+        $results = array_values($DB->get_records('search_index_requests', null, 'id'));
+        $this->assertCount(1, $results);
+        $this->assertEquals(101, $results[0]->indexpriority);
+
+        // Request forum 1, all areas, lower priority; adds second entry.
+        \core_search\manager::request_index($forum1ctx, '', 100);
+        $results = array_values($DB->get_records('search_index_requests', null, 'id'));
+        $this->assertCount(2, $results);
+        $this->assertEquals(100, $results[1]->indexpriority);
+
+        // Request course 1, all areas, lower priority; adds third entry.
+        \core_search\manager::request_index($course1ctx, '', 99);
+        $results = array_values($DB->get_records('search_index_requests', null, 'id'));
+        $this->assertCount(3, $results);
+        $this->assertEquals(99, $results[2]->indexpriority);
     }
 
     /**
@@ -969,14 +1002,15 @@ class search_manager_testcase extends advanced_testcase {
 
         // Hack the forums so they have different creation times.
         $now = time();
-        $DB->set_field('forum', 'timemodified', $now - 3, ['id' => $forum1->id]);
-        $DB->set_field('forum', 'timemodified', $now - 2, ['id' => $forum2->id]);
-        $DB->set_field('forum', 'timemodified', $now - 1, ['id' => $forum3->id]);
-        $forum2time = $now - 2;
+        $DB->set_field('forum', 'timemodified', $now - 30, ['id' => $forum1->id]);
+        $DB->set_field('forum', 'timemodified', $now - 20, ['id' => $forum2->id]);
+        $DB->set_field('forum', 'timemodified', $now - 10, ['id' => $forum3->id]);
+        $forum2time = $now - 20;
 
         // Make 2 index requests.
+        testable_core_search::fake_current_time($now - 3);
         $search::request_index(context_course::instance($course->id), 'mod_label-activity');
-        $this->waitForSecond();
+        testable_core_search::fake_current_time($now - 2);
         $search::request_index(context_module::instance($forum1->cmid));
 
         // Run with no time limit.
@@ -998,12 +1032,13 @@ class search_manager_testcase extends advanced_testcase {
         $this->assertEquals(0, $DB->count_records('search_index_requests'));
 
         // Request indexing the course a couple of times.
+        testable_core_search::fake_current_time($now - 3);
         $search::request_index(context_course::instance($course->id), 'mod_forum-activity');
+        testable_core_search::fake_current_time($now - 2);
         $search::request_index(context_course::instance($course->id), 'mod_forum-post');
 
         // Do the processing again with a time limit and indexing delay. The time limit is too
         // small; because of the way the logic works, this means it will index 2 activities.
-        testable_core_search::fake_current_time(time());
         $search->get_engine()->set_add_delay(0.2);
         $search->process_index_requests(0.1, $progress);
         $out = $progress->get_buffer();
@@ -1024,7 +1059,7 @@ class search_manager_testcase extends advanced_testcase {
         $this->assertEquals($forum2time, $records[0]->partialtime);
 
         // Run again and confirm it now finishes.
-        $search->process_index_requests(0.1, $progress);
+        $search->process_index_requests(2.0, $progress);
         $out = $progress->get_buffer();
         $progress->reset_buffer();
         $this->assertContains(
@@ -1036,5 +1071,26 @@ class search_manager_testcase extends advanced_testcase {
 
         // Confirm table is now empty.
         $this->assertEquals(0, $DB->count_records('search_index_requests'));
+
+        // Make 2 requests - first one is low priority.
+        testable_core_search::fake_current_time($now - 3);
+        $search::request_index(context_module::instance($forum1->cmid), 'mod_forum-activity',
+                \core_search\manager::INDEX_PRIORITY_REINDEXING);
+        testable_core_search::fake_current_time($now - 2);
+        $search::request_index(context_module::instance($forum2->cmid), 'mod_forum-activity');
+
+        // Process with short time limit and confirm it does the second one first.
+        $search->process_index_requests(0.1, $progress);
+        $out = $progress->get_buffer();
+        $progress->reset_buffer();
+        $this->assertContains(
+                'Completed requested context: Forum: TForum2 (search area: mod_forum-activity)',
+                $out);
+        $search->process_index_requests(0.1, $progress);
+        $out = $progress->get_buffer();
+        $progress->reset_buffer();
+        $this->assertContains(
+                'Completed requested context: Forum: TForum1 (search area: mod_forum-activity)',
+                $out);
     }
 }

version.php

@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2017122200.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2017122200.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.