From bb0651087fd4bce952ba0312b39ba083b198702a Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 22 Apr 2014 17:04:10 -0700 Subject: [PATCH] Use slave DB connections for LocalFile cache misses * Previously LocalFile cache misses would hit the master DB (even for shared repos like Commons). * Since lock() is always called before file changes, this has been modified to call a new markVolatile() method to track changing (or recently changed) files in memcached. On cache miss this tracking is used to decide whether to use the slave or the master DB. Change-Id: I942f5e28dd9e0953b6382cc2247ca480494d5718 --- includes/filerepo/file/LocalFile.php | 58 +++++++++++++++++++++++-- includes/filerepo/file/OldLocalFile.php | 3 +- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index dbf6918c0c..f89dc3d285 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -121,7 +121,13 @@ class LocalFile extends File { /** @var bool True if file is not present in file system. Not to be cached in memcached */ private $missing; + /** @var int UNIX timestamp of last markVolatile() call */ + private $lastMarkedVolatile = 0; + const LOAD_ALL = 1; // integer; load all the lazy fields too (like metadata) + const LOAD_VIA_SLAVE = 2; // integer; use a slave to load the data + + const VOLATILE_TTL = 300; // integer; seconds /** * Create a LocalFile from a title @@ -226,7 +232,7 @@ class LocalFile extends File { /** * Get the memcached key for the main data for this file, or false if * there is no access to the shared cache. - * @return bool + * @return string|bool */ function getCacheKey() { $hashedName = md5( $this->getName() ); @@ -374,7 +380,7 @@ class LocalFile extends File { /** * Load file metadata from the DB */ - function loadFromDB() { + function loadFromDB( $flags = 0 ) { # Polymorphic function name to distinguish foreign and local fetches $fname = get_class( $this ) . '::' . __FUNCTION__; wfProfileIn( $fname ); @@ -383,7 +389,10 @@ class LocalFile extends File { $this->dataLoaded = true; $this->extraDataLoaded = true; - $dbr = $this->repo->getMasterDB(); + $dbr = ( $flags & self::LOAD_VIA_SLAVE ) + ? $this->repo->getSlaveDB() + : $this->repo->getMasterDB(); + $row = $dbr->selectRow( 'image', $this->getCacheFields( 'img_' ), array( 'img_name' => $this->getName() ), $fname ); @@ -509,7 +518,7 @@ class LocalFile extends File { function load( $flags = 0 ) { if ( !$this->dataLoaded ) { if ( !$this->loadFromCache() ) { - $this->loadFromDB(); + $this->loadFromDB( $this->isVolatile() ? 0 : self::LOAD_VIA_SLAVE ); $this->saveToCache(); } $this->dataLoaded = true; @@ -1851,6 +1860,8 @@ class LocalFile extends File { } ); } + $this->markVolatile(); // file may change soon + return $dbw->selectField( 'image', '1', array( 'img_name' => $this->getName() ), __METHOD__, array( 'FOR UPDATE' ) ); } @@ -1870,6 +1881,45 @@ class LocalFile extends File { } } + /** + * Mark a file as about to be changed + * + * This sets a cache key that alters master/slave DB loading behavior + * + * @return bool Success + */ + protected function markVolatile() { + global $wgMemc; + + $key = $this->repo->getSharedCacheKey( 'file-volatile', md5( $this->getName() ) ); + if ( $key ) { + $this->lastMarkedVolatile = time(); + return $wgMemc->set( $key, $this->lastMarkedVolatile, self::VOLATILE_TTL ); + } + + return true; + } + + /** + * Check if a file is about to be changed or has been changed recently + * + * @see LocalFile::isVolatile() + * @return bool Whether the file is volatile + */ + protected function isVolatile() { + global $wgMemc; + + $key = $this->repo->getSharedCacheKey( 'file-volatile', md5( $this->getName() ) ); + if ( $key ) { + if ( $this->lastMarkedVolatile && ( time() - $this->lastMarkedVolatile ) <= self::VOLATILE_TTL ) { + return true; // sanity + } + return ( $wgMemc->get( $key ) !== false ); + } + + return false; + } + /** * Roll back the DB transaction and mark the image unlocked */ diff --git a/includes/filerepo/file/OldLocalFile.php b/includes/filerepo/file/OldLocalFile.php index d6c6a3c62f..ca9249654f 100644 --- a/includes/filerepo/file/OldLocalFile.php +++ b/includes/filerepo/file/OldLocalFile.php @@ -174,10 +174,11 @@ class OldLocalFile extends LocalFile { return $this->exists() && !$this->isDeleted( File::DELETED_FILE ); } - function loadFromDB() { + function loadFromDB( $flags = 0 ) { wfProfileIn( __METHOD__ ); $this->dataLoaded = true; + $dbr = $this->repo->getSlaveDB(); $conds = array( 'oi_name' => $this->getName() ); if ( is_null( $this->requestedTime ) ) { -- 2.20.1