Skip to content

Commit

Permalink
MDL-53325 search: Remove commit from engine interface
Browse files Browse the repository at this point in the history
Remove commit from engine, and instead notify when indexes
start and stop, allowing them to decide what to do.
  • Loading branch information
ericmerrill committed Mar 6, 2016
1 parent bf2235b commit 075fa91
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 33 deletions.
44 changes: 37 additions & 7 deletions search/classes/engine.php
Expand Up @@ -229,6 +229,18 @@ public function get_document_classname() {
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.
*
Expand All @@ -242,6 +254,31 @@ public function index_complete($numdocs = 0, $fullindex = false) {
// 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.
*
Expand Down Expand Up @@ -302,13 +339,6 @@ abstract function is_server_ready();
*/
abstract function add_document($doc);

/**
* Commits changes to the server.
*
* @return void
*/
abstract function commit();

/**
* Executes the query on the engine.
*
Expand Down
44 changes: 24 additions & 20 deletions search/classes/manager.php
Expand Up @@ -474,6 +474,9 @@ public function index($fullindex = false) {
// 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);
Expand All @@ -483,6 +486,9 @@ public function index($fullindex = false) {
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.
Expand Down Expand Up @@ -526,28 +532,28 @@ public function index($fullindex = false) {
$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);
}
}
}

Expand Down Expand Up @@ -608,7 +614,6 @@ public function delete_index($areaid = false) {
$this->engine->delete();
$this->reset_config();
}
$this->engine->commit();
}

/**
Expand All @@ -618,7 +623,6 @@ public function delete_index($areaid = false) {
*/
public function delete_index_by_id($id) {
$this->engine->delete_by_id($id);
$this->engine->commit();
}

/**
Expand Down
20 changes: 19 additions & 1 deletion search/engine/solr/classes/engine.php
Expand Up @@ -317,10 +317,26 @@ public function add_document($doc) {
*
* @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.
*
Expand All @@ -338,6 +354,7 @@ public function optimize() {
*/
public function delete_by_id($id) {
$this->get_search_client()->deleteById($id);
$this->commit();
}

/**
Expand All @@ -352,6 +369,7 @@ public function delete($areaid = null) {
} else {
$this->get_search_client()->deleteByQuery('*:*');
}
$this->commit();
}

/**
Expand Down
1 change: 0 additions & 1 deletion search/engine/solr/lang/en/search_solr.php
Expand Up @@ -29,7 +29,6 @@
$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';
Expand Down
4 changes: 0 additions & 4 deletions search/tests/fixtures/mock_search_engine.php
Expand Up @@ -41,10 +41,6 @@ public function add_document($doc) {
// No need to implement.
}

public function commit() {
// No need to implement.
}

public function execute_query($data, $usercontexts) {
// No need to implement.
}
Expand Down

0 comments on commit 075fa91

Please sign in to comment.