Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
When declaring all used caches backends in settings.php like that:
$conf['cache_backends'][] = 'sites/all/modules/mongodb/mongodb_cache/mongodb_cache.inc';
$conf['cache_backends'][] = 'sites/all/modules/apc/drupal_apc_cache.inc';
$conf['cache_backends'][] = 'sites/all/modules/redis/redis.autoload.inc';
The code used in mongodb_cache.inc will be executed before the code used in other backends, in _drupal_bootstrap_page_cache:
foreach (variable_get('cache_backends', array()) as $include)
{
require_once DRUPAL_ROOT . '/' . $include;
}
Now the first lines of this inc file are:
if (!drupal_load('module', 'mongodb')) {
include_once dirname(__FILE__) . '/../mongodb.module';
}
- Why testing the module load before doing an include_once, why not simply use a require_once and let apc opcode optimize require_once calls?
- drupal_load() will use drupal_get_filename which will make a cache_get(), and this cache_get may be using a different storage engine (like Redis) which is not yet lodaed in the _drupal_bootstrap_page_cache foreach loop => Crash, and maybe infinite loop if mongodb_cache is use for that cache.
A simple fix is to comment the if (not the include).
Comments
Comment #1
regilero CreditAttribution: regilero commentedAttached patch is simply commenting the if.
Comment #3
regilero CreditAttribution: regilero commentedPrevious patch was a 7.x-1.x patch, this one is really a 7.x-1.0-rc1 patch
Comment #4
regilero CreditAttribution: regilero commentedComment #6
chx CreditAttribution: chx commentedI fixed this by changing the condition to function_exists('mongodb'). Thanks for the report and the patch.
Comment #7
regilero CreditAttribution: regilero commentedOk,
But why even making a function exists, the goal of require_once is to avoid re-doing the require if it's already done, so PHP will do that check for you, and even more APC could do that for you, certainly faster than the function_exists check when millions of functions are loaded in memory?
Comment #8
chx CreditAttribution: chx commentedOne Zend hash lookup is not particularly expensive while doing file I/O might be.
Comment #9
regilero CreditAttribution: regilero commentedA require_once() call on an already included file will not make any file IO, AFAIK. Especially if you have an opcode like APC.
Comment #10
chx CreditAttribution: chx commentedOK. You made me read up on this. You should know that require_once was really badly inefficient in the past. On the other hand this has been fixed as long as PHP 5.2.0. So there's no more reason to avoid it, I will revert mine and commit yours.
Comment #11
regilero CreditAttribution: regilero commentedNice,
For people using PHP prior du 5.2 they'll have to use a good opcode, but I prefer having cleaner code, simple is beautiful.
Comment #12
chx CreditAttribution: chx commentedThey can't use PHP before 5.2.4 with D7 anyways.
Comment #13
Rok Žlender CreditAttribution: Rok Žlender commentedRerolled the patch according to comment in #10
Comment #15
marcingy CreditAttribution: marcingy commentedLooks good but needs a re-roll obviously
Comment #16
chx CreditAttribution: chx commentedThanks everyone.