Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge branch 'MDL-61102-master' of https://github.com/sammarshallou/m…
  • Loading branch information
David Monllao committed Apr 19, 2018
2 parents c805823 + 73fd566 commit 0299444
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 7 deletions.
33 changes: 33 additions & 0 deletions search/classes/engine.php
Expand Up @@ -74,6 +74,11 @@ abstract class engine {
*/
protected $pluginname = null;

/**
* @var bool If true, should skip schema validity check when checking the search engine is ready
*/
protected $skipschemacheck = false;

/**
* Initialises the search engine configuration.
*
Expand Down Expand Up @@ -417,10 +422,38 @@ public function clear_query_error() {
*
* This should also check that the search engine configuration is ok.
*
* If the function $this->should_skip_schema_check() returns true, then this function may leave
* out time-consuming checks that the schema is valid. (This allows for improved performance on
* critical pages such as the main search form.)
*
* @return true|string Returns true if all good or an error string.
*/
abstract function is_server_ready();

/**
* Tells the search engine to skip any time-consuming checks that it might do as part of the
* is_server_ready function, and only carry out a basic check that it can contact the server.
*
* This setting is not remembered and applies only to the current request.
*
* @since Moodle 3.5
* @param bool $skip True to skip the checks, false to start checking again
*/
public function skip_schema_check($skip = true) {
$this->skipschemacheck = $skip;
}

/**
* For use by subclasses. The engine can call this inside is_server_ready to check whether it
* should skip time-consuming schema checks.
*
* @since Moodle 3.5
* @return bool True if schema checks should be skipped
*/
protected function should_skip_schema_check() {
return $this->skipschemacheck;
}

/**
* Adds a document to the search engine.
*
Expand Down
36 changes: 35 additions & 1 deletion search/classes/manager.php
Expand Up @@ -133,15 +133,30 @@ public function __construct($engine) {
$this->engine = $engine;
}

/**
* @var int Record time of each successful schema check, but not more than once per 10 minutes.
*/
const SCHEMA_CHECK_TRACKING_DELAY = 10 * 60;

/**
* @var int Require a new schema check at least every 4 hours.
*/
const SCHEMA_CHECK_REQUIRED_EVERY = 4 * 3600;

/**
* Returns an initialised \core_search instance.
*
* While constructing the instance, checks on the search schema may be carried out. The $fast
* parameter provides a way to skip those checks on pages which are used frequently. It has
* no effect if an instance has already been constructed in this request.
*
* @see \core_search\engine::is_installed
* @see \core_search\engine::is_server_ready
* @param bool $fast Set to true when calling on a page that requires high performance
* @throws \core_search\engine_exception
* @return \core_search\manager
*/
public static function instance() {
public static function instance($fast = false) {
global $CFG;

// One per request, this should be purged during testing.
Expand All @@ -157,6 +172,17 @@ public static function instance() {
throw new \core_search\engine_exception('enginenotfound', 'search', '', $CFG->searchengine);
}

// Get time now and at last schema check.
$now = (int)self::get_current_time();
$lastschemacheck = get_config($engine->get_plugin_name(), 'lastschemacheck');

// On pages where performance matters, tell the engine to skip schema checks.
$skipcheck = false;
if ($fast && $now < $lastschemacheck + self::SCHEMA_CHECK_REQUIRED_EVERY) {
$skipcheck = true;
$engine->skip_schema_check();
}

if (!$engine->is_installed()) {
throw new \core_search\engine_exception('enginenotinstalled', 'search', '', $CFG->searchengine);
}
Expand All @@ -165,11 +191,19 @@ public static function instance() {
if ($serverstatus !== true) {
// Skip this error in Behat when faking seach results.
if (!defined('BEHAT_SITE_RUNNING') || !get_config('core_search', 'behat_fakeresult')) {
// Clear the record of successful schema checks since it might have failed.
unset_config('lastschemacheck', $engine->get_plugin_name());
// Error message with no details as this is an exception that any user may find if the server crashes.
throw new \core_search\engine_exception('engineserverstatus', 'search');
}
}

// If we did a successful schema check, record this, but not more than once per 10 minutes
// (to avoid updating the config db table/cache too often in case it gets called frequently).
if (!$skipcheck && $now >= $lastschemacheck + self::SCHEMA_CHECK_TRACKING_DELAY) {
set_config('lastschemacheck', $now, $engine->get_plugin_name());
}

static::$instance = new \core_search\manager($engine);
return static::$instance;
}
Expand Down
2 changes: 0 additions & 2 deletions search/classes/output/form/search.php
Expand Up @@ -70,8 +70,6 @@ function definition() {
$mform->addElement('text', 'title', get_string('title', 'search'));
$mform->setType('title', PARAM_TEXT);

$search = \core_search\manager::instance();

$searchareas = \core_search\manager::get_search_areas_list(true);
$areanames = array();
foreach ($searchareas as $areaid => $searcharea) {
Expand Down
6 changes: 6 additions & 0 deletions search/engine/solr/classes/engine.php
Expand Up @@ -1137,6 +1137,12 @@ public function is_server_ready() {
return $configured;
}

// As part of the above we have already checked that we can contact the server. For pages
// where performance is important, we skip doing a full schema check as well.
if ($this->should_skip_schema_check()) {
return true;
}

// Update schema if required/possible.
$schemalatest = $this->check_latest_schema();
if ($schemalatest !== true) {
Expand Down
2 changes: 1 addition & 1 deletion search/index.php
Expand Up @@ -54,7 +54,7 @@
exit;
}

$search = \core_search\manager::instance();
$search = \core_search\manager::instance(true);

// Set up custom data for form.
$customdata = ['searchengine' => $search->get_engine()->get_plugin_name()];
Expand Down
8 changes: 5 additions & 3 deletions search/upgrade.txt
Expand Up @@ -13,16 +13,13 @@ information provided here is intended especially for developers.
options (other than 'relevance' which is default). If there is more than one order then a choice
will be shown to users. (This is an optional feature, existing search engine plugins do not need
to be modified in order to continue working.)

* Module search areas that wish to support group filtering should set the new optional search
document field groupid (note: to remain compatible with earlier versions, do this inside an if
statement so that it only happens on 3.4+) and return true to the supports_group_restriction
function. See documentation in \core_search\base_mod class and example in \mod_forum\search\post.

* When a search engine supports group filtering, the \core_search\manager::search function now
accepts the optional 'groupids' parameter in its $data input. This parameter is an array of one
or more group IDs. If supplied, only results from those groups will be returned.

* Search engine plugins will need to be be modified if they wish to support group filtering.
(Search engines should continue to work unmodified, but will not then support group filtering.)
The modification steps are:
Expand All @@ -33,6 +30,11 @@ information provided here is intended especially for developers.
search results to specific groups) and the modified meaning of the second parameter,
$accessinfo (to automatically restrict search results users cannot access due to groups).
See implementation in Solr search engine.
* Search engine plugins can optionally use a new $this->should_skip_schema_check() function to
decide when to skip slow schema checking inside the is_server_ready function, improving user
performance on the search page. The Solr plugin implements this.
* API function \core_search\manager::instance() now includes a $fast parameter to skip schema
checks (as above).

=== 3.4 ===

Expand Down

0 comments on commit 0299444

Please sign in to comment.