Moodle

cron execution, improve deletion of cache_text records

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 1.9
  • Component/s: Administration
  • Labels:
    None
  • Database:
    Any
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

Under big sites, the deletion of old cache_text records can be really long.

I would propose to add one index to the timemodified field ASAP. I've tested that in a BIG-BIG site and differences are noticeable. It will cause a small delay on inserts, but I haven't been able to percept it.

Ciao

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Petr Škoda (skodak) added a comment -

if ($oldcacheitem = get_record_sql('SELECT * FROM '.$CFG->prefix.'cache_text WHERE md5key = \''.$md5key.'\'', true)) {
if ($oldcacheitem->timemodified >= $time) { return $oldcacheitem->formattedtext; }
}

I guess we could then use the timemodified in this query too - could be faster there too, right?

Show
Petr Škoda (skodak) added a comment - if ($oldcacheitem = get_record_sql('SELECT * FROM '.$CFG->prefix.'cache_text WHERE md5key = \''.$md5key.'\'', true)) { if ($oldcacheitem->timemodified >= $time) { return $oldcacheitem->formattedtext; } } I guess we could then use the timemodified in this query too - could be faster there too, right?
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

If indexed, yep (although shouldn't be really noticeable). Depends of the success/failure ratio.

But will save some traffic, for sure (until records are deleted by cron). We could add that, yep.

Show
Eloy Lafuente (stronk7) added a comment - If indexed, yep (although shouldn't be really noticeable). Depends of the success/failure ratio. But will save some traffic, for sure (until records are deleted by cron). We could add that, yep.
Hide
Permalink
Martin Dougiamas added a comment - - edited

hmm, there was some reason for this related to timing .... not sure right now what it was but just be really careful .... might be something to do with concurrency in a fast-changing table ... or better caching for mysql queries

Show
Martin Dougiamas added a comment - - edited hmm, there was some reason for this related to timing .... not sure right now what it was but just be really careful .... might be something to do with concurrency in a fast-changing table ... or better caching for mysql queries
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

yep, also.... be noted that some DBs are only able to une ONE index per table in one statement, so I guess the md5key one will continue being used always for individual fetching.

Although, the more I think about it, the more I think it can save a lot of traffic, resolving the timing directly under SQL (without sending the text to PHP).

What about:

1) Create the index. It, for sure will help cron deletion in a important degree.
2) Optionally, experiment in some real server with the two approaches (current one and the alternative, moving the timemodified condition to the SQL). Should be easy to measure the time spent by, say, 100.000 "if" blocks using each approach. Use those results to decide.

Waiting for answer...

Show
Eloy Lafuente (stronk7) added a comment - yep, also.... be noted that some DBs are only able to une ONE index per table in one statement, so I guess the md5key one will continue being used always for individual fetching. Although, the more I think about it, the more I think it can save a lot of traffic, resolving the timing directly under SQL (without sending the text to PHP). What about: 1) Create the index. It, for sure will help cron deletion in a important degree. 2) Optionally, experiment in some real server with the two approaches (current one and the alternative, moving the timemodified condition to the SQL). Should be easy to measure the time spent by, say, 100.000 "if" blocks using each approach. Use those results to decide. Waiting for answer...
Hide
Permalink
Petr Škoda (skodak) added a comment -

I do not like that code, the problem is that the get_record may return expired record even if fresh one exists, which would result in broken caching mechanism, maybe we could order it and return the latest one, ideas?

Show
Petr Škoda (skodak) added a comment - I do not like that code, the problem is that the get_record may return expired record even if fresh one exists, which would result in broken caching mechanism, maybe we could order it and return the latest one, ideas?
Hide
Permalink
Martin Dougiamas added a comment -

+2 for the index, sure!

Petr, for the other thing, I don't think it's a big danger as you'd need a really lucky combination of accesses to cause it and even then it has a limited lifespan. Some discussion at MDL-4677.

Show
Martin Dougiamas added a comment - +2 for the index, sure! Petr, for the other thing, I don't think it's a big danger as you'd need a really lucky combination of accesses to cause it and even then it has a limited lifespan. Some discussion at MDL-4677.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Index added for the timemodifed field. The table will be truncated to prevent long indexing times.

About to move the time condition to the select query... I've a bit lost, should we try it or no?

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Index added for the timemodifed field. The table will be truncated to prevent long indexing times. About to move the time condition to the select query... I've a bit lost, should we try it or no? Ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Is this close-able once the index has been implemented? Or should we advance in all/some of the ideas exposed above:

1- adding the time condition to SQL to save some (a lot!) traffic.
2- retrieving ordered by time desc in order to have a better probability to get fresh records.

Alternatives are:

0- close
1- do 1
2- do 2
3- do 1 & 2

Ciao :-P

Show
Eloy Lafuente (stronk7) added a comment - Is this close-able once the index has been implemented? Or should we advance in all/some of the ideas exposed above: 1- adding the time condition to SQL to save some (a lot!) traffic. 2- retrieving ordered by time desc in order to have a better probability to get fresh records. Alternatives are: 0- close 1- do 1 2- do 2 3- do 1 & 2 Ciao :-P
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Plz, 0...4.

TIA, ciao

Show
Eloy Lafuente (stronk7) added a comment - Plz, 0...4. TIA, ciao
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Roger.

No answer, no further action. Closing this now! :-P

Show
Eloy Lafuente (stronk7) added a comment - Roger. No answer, no further action. Closing this now! :-P

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.5#665-sha1:422aead) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.