Moodle

rcache caching empty records

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 1.8.4, 1.9, 2.0
  • Component/s: Performance
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE

Description

I was seeing some warnings about recent clone changes:
Warning: __clone method called on non-object in /var/www/moodle/lib/dmllib.php on line 2586

And realised it was to do with empty stuff being sent to rcache_set().

I thought I was going crazy because:
$foo = get_record('course', 'id', '99999999999');

trigered the warning but this didn't:
$foo = get_record('course', 'shortname', '99999999999');

Then I realised $docache is only set when id is the first record.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Dan Poltawski added a comment -

Our rcache test for empty records was incorrect as $record was set but false when no records returned.

Fixed in CVS

Show
Dan Poltawski added a comment - Our rcache test for empty records was incorrect as $record was set but false when no records returned. Fixed in CVS
Hide
Permalink
Martín Langhoff added a comment -

Ah - great diagnosis - I had seen a couple such warnings but couldn't repro them. Now I know why. Thanks.!

Show
Martín Langhoff added a comment - Ah - great diagnosis - I had seen a couple such warnings but couldn't repro them. Now I know why. Thanks.!
Hide
Permalink
Mark Nielsen added a comment -

Attached to MDL-12827, I proposed a different solution to the same problem where it would still cache the false and thus prevent going to the database again on the same query and use the cache instead. Just a heads up.

Show
Mark Nielsen added a comment - Attached to MDL-12827, I proposed a different solution to the same problem where it would still cache the false and thus prevent going to the database again on the same query and use the cache instead. Just a heads up.
Hide
Permalink
Martín Langhoff added a comment -

Hi Mark,

2 comments on your patch...

  • Unfortunately some of the rcache backends (apc, eaccelerator and perhaps memched) cannot store a 'false' value. We could workaround this but it wouldn't be pretty.
  • I do not think there is any benefit to caching a 'false', we only cache when the search was by id, and an id we know to search for is not there, it usually translates into an error condition. Have you got any cases where your patch speeds things up / saves queries?
Show
Martín Langhoff added a comment - Hi Mark, 2 comments on your patch...
  • Unfortunately some of the rcache backends (apc, eaccelerator and perhaps memched) cannot store a 'false' value. We could workaround this but it wouldn't be pretty.
  • I do not think there is any benefit to caching a 'false', we only cache when the search was by id, and an id we know to search for is not there, it usually translates into an error condition. Have you got any cases where your patch speeds things up / saves queries?

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.