Skip to content

Commit

Permalink
MDL-34347 remove all problematic global text caching
Browse files Browse the repository at this point in the history
Instead we will create new MUC caches inside each filter plugin.
Please note that all cache filters should work with local caches
without the need of strict cache invalidation.
  • Loading branch information
skodak committed Jan 14, 2014
1 parent 12efa52 commit 62c8032
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 137 deletions.
24 changes: 0 additions & 24 deletions admin/settings/plugins.php
Expand Up @@ -142,31 +142,7 @@
// "filtersettings" settingpage
$temp = new admin_settingpage('commonfiltersettings', new lang_string('commonfiltersettings', 'admin'));
if ($ADMIN->fulltree) {
$cachetimes = array(
604800 => new lang_string('numdays','',7),
86400 => new lang_string('numdays','',1),
43200 => new lang_string('numhours','',12),
10800 => new lang_string('numhours','',3),
7200 => new lang_string('numhours','',2),
3600 => new lang_string('numhours','',1),
2700 => new lang_string('numminutes','',45),
1800 => new lang_string('numminutes','',30),
900 => new lang_string('numminutes','',15),
600 => new lang_string('numminutes','',10),
540 => new lang_string('numminutes','',9),
480 => new lang_string('numminutes','',8),
420 => new lang_string('numminutes','',7),
360 => new lang_string('numminutes','',6),
300 => new lang_string('numminutes','',5),
240 => new lang_string('numminutes','',4),
180 => new lang_string('numminutes','',3),
120 => new lang_string('numminutes','',2),
60 => new lang_string('numminutes','',1),
30 => new lang_string('numseconds','',30),
0 => new lang_string('no')
);
$items = array();
$items[] = new admin_setting_configselect('cachetext', new lang_string('cachetext', 'admin'), new lang_string('configcachetext', 'admin'), 60, $cachetimes);
$items[] = new admin_setting_configselect('filteruploadedfiles', new lang_string('filteruploadedfiles', 'admin'), new lang_string('configfilteruploadedfiles', 'admin'), 0,
array('0' => new lang_string('none'), '1' => new lang_string('allfiles'), '2' => new lang_string('htmlfilesonly')));
$items[] = new admin_setting_configcheckbox('filtermatchoneperpage', new lang_string('filtermatchoneperpage', 'admin'), new lang_string('configfiltermatchoneperpage', 'admin'), 0);
Expand Down
6 changes: 6 additions & 0 deletions filter/upgrade.txt
@@ -1,6 +1,12 @@
This file describes API changes in core filter API and plugins,
information provided here is intended especially for developers.

=== 2.7 ===

* Finally filter may use $PAGE and $OUTPUT, yay!
* Old global text caching was removed, each filter is now responsible
for own caching.

=== 2.6 ===

* filtersettings.php is now deprecated, migrate to standard settings.php
Expand Down
2 changes: 0 additions & 2 deletions lang/en/admin.php
Expand Up @@ -92,7 +92,6 @@
$string['bookmarkthispage'] = 'Bookmark this page';
$string['cachejs'] = 'Cache Javascript';
$string['cachejs_help'] = 'Javascript caching and compression greatly improves page loading performance. it is strongly recommended for production sites. Developers will probably want to disable this feature.';
$string['cachetext'] = 'Text cache lifetime';
$string['calendarexportsalt'] = 'Calendar export salt';
$string['calendarsettings'] = 'Calendar';
$string['calendartype'] = 'Calendar type';
Expand Down Expand Up @@ -147,7 +146,6 @@
$string['configautolang'] = 'Detect default language from browser setting, if disabled site default is used.';
$string['configautologinguests'] = 'Should visitors be logged in as guests automatically when entering courses with guest access?';
$string['configbloglevel'] = 'This setting allows you to restrict the level to which user blogs can be viewed on this site. Note that they specify the maximum context of the VIEWER not the poster or the types of blog posts. Blogs can also be disabled completely if you don\'t want them at all.';
$string['configcachetext'] = 'For larger sites or sites that use text filters, this setting can really speed things up. Copies of texts will be retained in their processed form for the time specified here. Setting this too small may actually slow things down slightly, but setting it too large may mean texts take too long to refresh (with new links, for example).';
$string['configcalendarcustomexport'] = 'Enable custom date range export of calendar';
$string['configcalendarexportsalt'] = 'This random text is used for improving of security of authentication tokens used for exporting of calendars. Please note that all current tokens are invalidated if you change this hash salt.';
$string['configclamactlikevirus'] = 'Treat files like viruses';
Expand Down
8 changes: 0 additions & 8 deletions lib/cronlib.php
Expand Up @@ -133,14 +133,6 @@ function cron_run() {
}


// Delete old cached texts
if (!empty($CFG->cachetext)) { // Defined in config.php
$cachelifetime = time() - $CFG->cachetext - 60; // Add an extra minute to allow for really heavy sites
$DB->delete_records_select('cache_text', "timemodified < ?", array($cachelifetime));
mtrace(" Deleted old cache_text records");
}


if (!empty($CFG->usetags)) {
require_once($CFG->dirroot.'/tag/lib.php');
tag_cron();
Expand Down
17 changes: 1 addition & 16 deletions lib/db/install.xml
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8" ?>
<XMLDB PATH="lib/db" VERSION="20131009" COMMENT="XMLDB file for core Moodle tables"
<XMLDB PATH="lib/db" VERSION="20140112" COMMENT="XMLDB file for core Moodle tables"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
>
Expand Down Expand Up @@ -532,21 +532,6 @@
<INDEX NAME="filter_md5key" UNIQUE="false" FIELDS="filter, md5key"/>
</INDEXES>
</TABLE>
<TABLE NAME="cache_text" COMMENT="For storing temporary copies of processed texts">
<FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
<FIELD NAME="md5key" TYPE="char" LENGTH="32" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="formattedtext" TYPE="text" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id"/>
</KEYS>
<INDEXES>
<INDEX NAME="md5key" UNIQUE="false" FIELDS="md5key"/>
<INDEX NAME="timemodified" UNIQUE="false" FIELDS="timemodified" COMMENT="Mainly to help deletion of expired records from cron"/>
</INDEXES>
</TABLE>
<TABLE NAME="log" COMMENT="Every action is logged as far as possible">
<FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
Expand Down
16 changes: 16 additions & 0 deletions lib/db/upgrade.php
Expand Up @@ -2903,5 +2903,21 @@ function xmldb_main_upgrade($oldversion) {
upgrade_main_savepoint(true, 2013122400.01);
}

if ($oldversion < 2014011000.01) {

// Define table cache_text to be dropped.
$table = new xmldb_table('cache_text');

// Conditionally launch drop table for cache_text.
if ($dbman->table_exists($table)) {
$dbman->drop_table($table);
}

unset_config('cachetext');

// Main savepoint reached.
upgrade_main_savepoint(true, 2014011000.01);
}

return true;
}
1 change: 0 additions & 1 deletion lib/phpunit/bootstrap.php
Expand Up @@ -205,7 +205,6 @@
ini_set('log_errors', '1');

$CFG->noemailever = true; // better not mail anybody from tests, override temporarily if necessary
$CFG->cachetext = 0; // disable this very nasty setting

// some ugly hacks
$CFG->themerev = 1;
Expand Down
89 changes: 4 additions & 85 deletions lib/weblib.php
Expand Up @@ -1097,7 +1097,6 @@ function format_text_menu() {
*/
function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseiddonotuse = null) {
global $CFG, $DB, $PAGE;
static $croncache = array();

if ($text === '' || is_null($text)) {
// No need to do any filters and cleaning.
Expand Down Expand Up @@ -1166,34 +1165,6 @@ function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseidd
$filtermanager = new null_filter_manager();
}

if (!empty($CFG->cachetext) and empty($options['nocache'])) {
$hashstr = $text.'-'.$filtermanager->text_filtering_hash($context).'-'.$context->id.'-'.current_language().'-'.
(int)$format.(int)$options['trusted'].(int)$options['noclean'].
(int)$options['para'].(int)$options['newlines'];

$time = time() - $CFG->cachetext;
$md5key = md5($hashstr);
if (CLI_SCRIPT) {
if (isset($croncache[$md5key])) {
return $croncache[$md5key];
}
}

if ($oldcacheitem = $DB->get_record('cache_text', array('md5key' => $md5key), '*', IGNORE_MULTIPLE)) {
if ($oldcacheitem->timemodified >= $time) {
if (CLI_SCRIPT) {
if (count($croncache) > 150) {
reset($croncache);
$key = key($croncache);
unset($croncache[$key]);
}
$croncache[$md5key] = $oldcacheitem->formattedtext;
}
return $oldcacheitem->formattedtext;
}
}
}

switch ($format) {
case FORMAT_HTML:
if (!$options['noclean']) {
Expand Down Expand Up @@ -1257,65 +1228,15 @@ function format_text($text, $format = FORMAT_MOODLE, $options = null, $courseidd
}
}

// Warn people that we have removed this old mechanism, just in case they
// were stupid enough to rely on it.
if (isset($CFG->currenttextiscacheable)) {
debugging('Once upon a time, Moodle had a truly evil use of global variables ' .
'called $CFG->currenttextiscacheable. The good news is that this no ' .
'longer exists. The bad news is that you seem to be using a filter that '.
'relies on it. Please seek out and destroy that filter code.', DEBUG_DEVELOPER);
}

if (!empty($options['overflowdiv'])) {
$text = html_writer::tag('div', $text, array('class' => 'no-overflow'));
}

if (empty($options['nocache']) and !empty($CFG->cachetext)) {
if (CLI_SCRIPT) {
// Special static cron cache - no need to store it in db if its not already there.
if (count($croncache) > 150) {
reset($croncache);
$key = key($croncache);
unset($croncache[$key]);
}
$croncache[$md5key] = $text;
return $text;
}

$newcacheitem = new stdClass();
$newcacheitem->md5key = $md5key;
$newcacheitem->formattedtext = $text;
$newcacheitem->timemodified = time();
if ($oldcacheitem) {
// See bug 4677 for discussion.
$newcacheitem->id = $oldcacheitem->id;
try {
// Update existing record in the cache table.
$DB->update_record('cache_text', $newcacheitem);
} catch (dml_exception $e) {
// It's unlikely that the cron cache cleaner could have
// deleted this entry in the meantime, as it allows
// some extra time to cover these cases.
}
} else {
try {
// Insert a new record in the cache table.
$DB->insert_record('cache_text', $newcacheitem);
} catch (dml_exception $e) {
// Again, it's possible that another user has caused this
// record to be created already in the time that it took
// to traverse this function. That's OK too, as the
// call above handles duplicate entries, and eventually
// the cron cleaner will delete them.
}
}
}

return $text;
}

/**
* Resets all data related to filters, called during upgrade or when filter settings change.
* Resets some data related to filters, called during upgrade or when general filter settings change.
*
* @param bool $phpunitreset true means called from our PHPUnit integration test reset
* @return void
Expand All @@ -1325,14 +1246,12 @@ function reset_text_filters_cache($phpunitreset = false) {

if ($phpunitreset) {
// HTMLPurifier does not change, DB is already reset to defaults,
// nothing to do here.
// nothing to do here, the dataroot was cleared too.
return;
}

$DB->delete_records('cache_text');

$purifdir = $CFG->cachedir.'/htmlpurifier';
remove_dir($purifdir, true);
// The purge_all_caches() deals with cachedir and localcachedir purging,
// the individual filter caches are invalidated as necessary elsewhere.

// Update $CFG->filterall cache flag.
if (empty($CFG->stringfilters)) {
Expand Down
2 changes: 1 addition & 1 deletion version.php
Expand Up @@ -29,7 +29,7 @@

defined('MOODLE_INTERNAL') || die();

$version = 2014011000.00; // YYYYMMDD = weekly release date of this DEV branch.
$version = 2014011000.01; // YYYYMMDD = weekly release date of this DEV branch.
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.

Expand Down

0 comments on commit 62c8032

Please sign in to comment.