Page MenuHomePhabricator

Article class should use Article->getDB consistently
Closed, ResolvedPublic

Description

The Article class has a function called getDB which returns wfGetDB( DB_MASTER ). Its called 3 times or so in the code, but there are still alot of places where its not used and wfGetDB is called directly. The class should be consistent one way or the other in using this function.


Version: 1.13.x
Severity: enhancement
URL: http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/includes/Article.php?view=diff&r1=19597&r2=19598

Details

Reference
bz13878

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:14 PM
bzimport set Reference to bz13878.
bzimport added a subscriber: Unknown Object (MLST).

This method originally chose _either_ a master or slave, depending on whether things were going to be written to or not. This ended up being kind of weird and unreliable; data pulled from slaves was relatively likely to end up out-of-date shortly after an edit due to replication lag, which then polluted caches and generally caused trouble.

This was thus hacked to always hit the master for better reliability, and the hack ended up in core.

Since revision and text data are now in separate tables, these days we pull most of _that_ from slaves while getting the latest article metadata (name, cache invalidation timestamp, and latest revision number) from the master, always up to date, with relatively little load to the master as it's a simple row load.

Article::getDB() should itself be phased out, as it no longer serves any purpose in selecting a database connection.

On line 74 we have:

$dbr = wfGetDB(DB_SLAVE);

Currently, getDB member function doesn't accept a parameter and only connects to DB_MASTER. We can't use it for above line, unless we either accept to connect to DB_MASTER instead of DB_SLAVE (!) or change getDB to accept parameters (and then what is the difference between using it and using wfGetDB?!)

So, I agree with what brion said (and caused a mid-air collision while I was sending this!)

Last of Article::getDB() phased out in r36334. The method has been deprecated, but retains the old functionality, just in case someone out there is using it in an extension.