User "Catrope" posted a comment on MediaWiki.r96837.

Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/96837#c23717
Commit summary:

RL2: Reorganize caching in LocalGadgetRepo so we no longer load the entire 
gadgets table from the DB on every page view. There is now one memcached entry 
per gadget, as well as an entry for the list of names.

* Rename loadData() to loadIDs() and add loadDataFor(). These functions pull 
info from 1) $this->data, 2) memcached, 3) DB and propagate back up the chain 
when misses occur
* Add getMemcKey() à la ForeignDBRepo (in phase3/includes/filerepo) and finally 
uncomment hasSharedCache. This means we now take advantage of cross-wiki 
memcached access for WMF-like configurations
* Put in a dirty hack to prevent cache pollution of the names list between 
LocalGadgetRepo and ForeignDBGadgetRepo. Will fix this properly later.

Comment:

I didn't understand the bug description at first, but after reproducing the bug 
myself, examining the stack trace and var_dump()ing something, I now understand 
what's happening:

* Someone calls getGadgetIds(), which calls loadIDs(), which populates 
$this->data and sets $this->idsLoaded = true;
* Someone calls getGadget( 'babla' ), which calls loadDataFor( 'babla' ), which 
queries the DB, finds it missing, and sets $this->data['babla'] = array();
* Someone calls getGadgetIds() again, which sees $this->idsLoaded = true; and 
returns array_keys( $this->data ), which includes 'babla'
* The caller assumes that nothing returned by getGadgetIds() will ever return 
null when fed into getGadget(), which should be a safe assumption, but explodes 
here.

I'm on it.

_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview

Reply via email to