From 075fa912718cc2e4f8255ffe2d88bc9d65f9e559 Mon Sep 17 00:00:00 2001 From: Eric Merrill Date: Thu, 3 Mar 2016 22:06:49 -0500 Subject: [PATCH] MDL-53325 search: Remove commit from engine interface Remove commit from engine, and instead notify when indexes start and stop, allowing them to decide what to do. --- search/classes/engine.php | 44 ++++++++++++++++---- search/classes/manager.php | 44 +++++++++++--------- search/engine/solr/classes/engine.php | 20 ++++++++- search/engine/solr/lang/en/search_solr.php | 1 - search/tests/fixtures/mock_search_engine.php | 4 -- 5 files changed, 80 insertions(+), 33 deletions(-) diff --git a/search/classes/engine.php b/search/classes/engine.php index d2097feb62f..9711196b02a 100644 --- a/search/classes/engine.php +++ b/search/classes/engine.php @@ -229,6 +229,18 @@ abstract class engine { return $classname; } + /** + * Run any pre-indexing operations. + * + * Should be overwritten if the search engine needs to do any pre index preparation. + * + * @param bool $fullindex True if a full index will be performed + * @return void + */ + public function index_starting($fullindex = false) { + // Nothing by default. + } + /** * Run any post indexing operations. * @@ -242,6 +254,31 @@ abstract class engine { // Nothing by default. } + /** + * Do anything that may need to be done before an area is indexed. + * + * @param \core_search\area\base $searcharea The search area that was complete + * @param bool $fullindex True if a full index is being performed + * @return void + */ + public function area_index_starting($searcharea, $fullindex = false) { + // Nothing by default. + } + + /** + * Do any area cleanup needed, and do anything to confirm contents. + * + * Return false to prevent the search area completed time and stats from being updated. + * + * @param \core_search\area\base $searcharea The search area that was complete + * @param int $numdocs The number of documents that were added to the index + * @param bool $fullindex True if a full index is being performed + * @return bool True means that data is considered indexed + */ + public function area_index_complete($searcharea, $numdocs = 0, $fullindex = false) { + return true; + } + /** * Optimizes the search engine. * @@ -302,13 +339,6 @@ abstract class engine { */ abstract function add_document($doc); - /** - * Commits changes to the server. - * - * @return void - */ - abstract function commit(); - /** * Executes the query on the engine. * diff --git a/search/classes/manager.php b/search/classes/manager.php index 0437e24df85..3a569610778 100644 --- a/search/classes/manager.php +++ b/search/classes/manager.php @@ -474,6 +474,9 @@ class manager { // Unlimited time. \core_php_time_limit::raise(); + // Notify the engine that an index starting. + $this->engine->index_starting($fullindex); + $sumdocs = 0; $searchareas = $this->get_search_areas_list(true); @@ -483,6 +486,9 @@ class manager { mtrace('Processing ' . $searcharea->get_visible_name() . ' area'); } + // Notify the engine that an area is starting. + $this->engine->area_index_starting($searcharea, $fullindex); + $indexingstart = time(); // This is used to store this component config. @@ -526,28 +532,28 @@ class manager { $numrecords++; } - if ($numdocs > 0) { - $sumdocs += $numdocs; - - // Commit all remaining documents. - $this->engine->commit(); - - if (CLI_SCRIPT && !PHPUNIT_TEST) { + if (CLI_SCRIPT && !PHPUNIT_TEST) { + if ($numdocs > 0) { mtrace('Processed ' . $numrecords . ' records containing ' . $numdocs . ' documents for ' . - $searcharea->get_visible_name() . ' area. Commits completed.'); + $searcharea->get_visible_name() . ' area.'); + } else { + mtrace('No new documents to index for ' . $searcharea->get_visible_name() . ' area.'); } - } else if (CLI_SCRIPT && !PHPUNIT_TEST) { - mtrace('No new documents to index for ' . $searcharea->get_visible_name() . ' area.'); } - // Store last index run once documents have been commited to the search engine. - set_config($varname . '_indexingstart', $indexingstart, $componentconfigname); - set_config($varname . '_indexingend', time(), $componentconfigname); - set_config($varname . '_docsignored', $numdocsignored, $componentconfigname); - set_config($varname . '_docsprocessed', $numdocs, $componentconfigname); - set_config($varname . '_recordsprocessed', $numrecords, $componentconfigname); - if ($lastindexeddoc > 0) { - set_config($varname . '_lastindexrun', $lastindexeddoc, $componentconfigname); + // Notify the engine this area is complete, and only mark times if true. + if ($this->engine->area_index_complete($searcharea, $numdocs, $fullindex)) { + $sumdocs += $numdocs; + + // Store last index run once documents have been commited to the search engine. + set_config($varname . '_indexingstart', $indexingstart, $componentconfigname); + set_config($varname . '_indexingend', time(), $componentconfigname); + set_config($varname . '_docsignored', $numdocsignored, $componentconfigname); + set_config($varname . '_docsprocessed', $numdocs, $componentconfigname); + set_config($varname . '_recordsprocessed', $numrecords, $componentconfigname); + if ($lastindexeddoc > 0) { + set_config($varname . '_lastindexrun', $lastindexeddoc, $componentconfigname); + } } } @@ -608,7 +614,6 @@ class manager { $this->engine->delete(); $this->reset_config(); } - $this->engine->commit(); } /** @@ -618,7 +623,6 @@ class manager { */ public function delete_index_by_id($id) { $this->engine->delete_by_id($id); - $this->engine->commit(); } /** diff --git a/search/engine/solr/classes/engine.php b/search/engine/solr/classes/engine.php index 1e45829519d..062017370fc 100644 --- a/search/engine/solr/classes/engine.php +++ b/search/engine/solr/classes/engine.php @@ -317,10 +317,26 @@ class engine extends \core_search\engine { * * @return void */ - public function commit() { + protected function commit() { $this->get_search_client()->commit(); } + /** + * Do any area cleanup needed, and do anything to confirm contents. + * + * Return false to prevent the search area completed time and stats from being updated. + * + * @param \core_search\area\base $searcharea The search area that was complete + * @param int $numdocs The number of documents that were added to the index + * @param bool $fullindex True if a full index is being performed + * @return bool True means that data is considered indexed + */ + public function area_index_complete($searcharea, $numdocs = 0, $fullindex = false) { + $this->commit(); + + return true; + } + /** * Defragments the index. * @@ -338,6 +354,7 @@ class engine extends \core_search\engine { */ public function delete_by_id($id) { $this->get_search_client()->deleteById($id); + $this->commit(); } /** @@ -352,6 +369,7 @@ class engine extends \core_search\engine { } else { $this->get_search_client()->deleteByQuery('*:*'); } + $this->commit(); } /** diff --git a/search/engine/solr/lang/en/search_solr.php b/search/engine/solr/lang/en/search_solr.php index e82b7558e30..9869aa5d72f 100644 --- a/search/engine/solr/lang/en/search_solr.php +++ b/search/engine/solr/lang/en/search_solr.php @@ -29,7 +29,6 @@ $string['extensionerror'] = 'The Apache Solr PHP extension is not installed. Ple $string['missingconfig'] = 'Your Apache Solr server is not yet configured in Moodle.'; $string['multivaluedfield'] = 'Field "{$a}" returned an array instead of a scalar, the field is probably defined in Solr with "Multivalued" to true, this means that Solr autocreated the field for you when you indexed data because you forgot to run search/engine/solr/cli/setup_schema.php. Please delete the current index, create a new one and run setup_schema.php before indexing data in Solr.'; $string['nodatafromserver'] = 'No data from server'; -$string['optimizetask'] = 'Optimize Solr index'; $string['pluginname'] = 'Solr'; $string['schemafieldautocreated'] = 'Field "{$a}" already exists in Solr schema. You probably forgot to run this script before indexing data and fields were autocreated by Solr. Please delete the current index, create a new one and run setup_schema.php again before indexing data in Solr.'; $string['searchinfo'] = 'Search queries'; diff --git a/search/tests/fixtures/mock_search_engine.php b/search/tests/fixtures/mock_search_engine.php index a56bda88251..0021d951722 100644 --- a/search/tests/fixtures/mock_search_engine.php +++ b/search/tests/fixtures/mock_search_engine.php @@ -41,10 +41,6 @@ class engine extends \core_search\engine { // No need to implement. } - public function commit() { - // No need to implement. - } - public function execute_query($data, $usercontexts) { // No need to implement. }