From 8288b34eaede7dd80a54a86dbde9f58ab6afd9a8 Mon Sep 17 00:00:00 2001 From: Aaron Date: Tue, 22 May 2012 18:27:29 -0700 Subject: [PATCH] Reduced some master queries by adding flags to Revision functions. * The main Revision functions now allow various QoS and locking flags. * Added tiny DBAO interface add made Revision implement it. Since a lot of objects will need (or have) the same functionality. * Use "self" keyword in Revision class consistently. * Made Revisison::newFromConds() private. Change-Id: I3139956999218a2bb44b5c845b8079e33b2328bb --- includes/AutoLoader.php | 3 + includes/Revision.php | 89 +++++++++++++++--------- includes/dao/IDBAccessObject.php | 32 +++++++++ includes/diff/DifferenceEngine.php | 4 +- includes/filerepo/file/LocalFile.php | 2 +- includes/search/SearchEngine.php | 3 +- includes/specials/SpecialBooksources.php | 2 +- 7 files changed, 99 insertions(+), 36 deletions(-) create mode 100644 includes/dao/IDBAccessObject.php diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 27ecc20c0e..7b5b56b532 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -427,6 +427,9 @@ $wgAutoloadLocalClasses = array( 'IContextSource' => 'includes/context/IContextSource.php', 'RequestContext' => 'includes/context/RequestContext.php', + # includes/dao + 'IDBAccessObject' => 'includes/dao/IDBAccessObject.php', + # includes/db 'Blob' => 'includes/db/DatabaseUtility.php', 'ChronologyProtector' => 'includes/db/LBFactory.php', diff --git a/includes/Revision.php b/includes/Revision.php index 6928eb97f6..f2862a8a99 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -23,7 +23,7 @@ /** * @todo document */ -class Revision { +class Revision implements IDBAccessObject { protected $mId; protected $mPage; protected $mUserText; @@ -47,7 +47,7 @@ class Revision { const DELETED_RESTRICTED = 8; // Convenience field const SUPPRESSED_USER = 12; - // Audience options for Revision::getText() + // Audience options for accessors const FOR_PUBLIC = 1; const FOR_THIS_USER = 2; const RAW = 3; @@ -56,11 +56,17 @@ class Revision { * Load a page revision from a given revision ID number. * Returns null if no such revision can be found. * + * $flags include: + * IDBAccessObject::LATEST_READ : Select the data from the master + * IDBAccessObject::LOCKING_READ : Select & lock the data from the master + * IDBAccessObject::AVOID_MASTER : Avoid master queries; data may be stale + * * @param $id Integer + * @param $flags Integer (optional) * @return Revision or null */ - public static function newFromId( $id ) { - return Revision::newFromConds( array( 'rev_id' => intval( $id ) ) ); + public static function newFromId( $id, $flags = 0 ) { + return self::newFromConds( array( 'rev_id' => intval( $id ) ), $flags ); } /** @@ -68,11 +74,17 @@ class Revision { * that's attached to a given title. If not attached * to that title, will return null. * + * $flags include: + * IDBAccessObject::LATEST_READ : Select the data from the master + * IDBAccessObject::LOCKING_READ : Select & lock the data from the master + * IDBAccessObject::AVOID_MASTER : Avoid master queries; data may be stale + * * @param $title Title * @param $id Integer (optional) + * @param $flags Integer Bitfield (optional) * @return Revision or null */ - public static function newFromTitle( $title, $id = 0 ) { + public static function newFromTitle( $title, $id = 0, $flags = 0 ) { $conds = array( 'page_namespace' => $title->getNamespace(), 'page_title' => $title->getDBkey() @@ -80,7 +92,7 @@ class Revision { if ( $id ) { // Use the specified ID $conds['rev_id'] = $id; - } elseif ( wfGetLB()->getServerCount() > 1 ) { + } elseif ( !( $flags & self::AVOID_MASTER ) && wfGetLB()->getServerCount() > 1 ) { // Get the latest revision ID from the master $dbw = wfGetDB( DB_MASTER ); $latest = $dbw->selectField( 'page', 'page_latest', $conds, __METHOD__ ); @@ -92,7 +104,7 @@ class Revision { // Use a join to get the latest revision $conds[] = 'rev_id=page_latest'; } - return Revision::newFromConds( $conds ); + return self::newFromConds( $conds, $flags ); } /** @@ -100,15 +112,21 @@ class Revision { * that's attached to a given page ID. * Returns null if no such revision can be found. * + * $flags include: + * IDBAccessObject::LATEST_READ : Select the data from the master + * IDBAccessObject::LOCKING_READ : Select & lock the data from the master + * IDBAccessObject::AVOID_MASTER : Avoid master queries; data may be stale + * * @param $revId Integer * @param $pageId Integer (optional) + * @param $flags Integer Bitfield (optional) * @return Revision or null */ - public static function newFromPageId( $pageId, $revId = 0 ) { + public static function newFromPageId( $pageId, $revId = 0, $flags = 0 ) { $conds = array( 'page_id' => $pageId ); if ( $revId ) { $conds['rev_id'] = $revId; - } elseif ( wfGetLB()->getServerCount() > 1 ) { + } elseif ( !( $flags & self::AVOID_MASTER ) && wfGetLB()->getServerCount() > 1 ) { // Get the latest revision ID from the master $dbw = wfGetDB( DB_MASTER ); $latest = $dbw->selectField( 'page', 'page_latest', $conds, __METHOD__ ); @@ -119,7 +137,7 @@ class Revision { } else { $conds[] = 'rev_id = page_latest'; } - return Revision::newFromConds( $conds ); + return self::newFromConds( $conds, $flags ); } /** @@ -175,7 +193,7 @@ class Revision { * @return Revision or null */ public static function loadFromId( $db, $id ) { - return Revision::loadFromConds( $db, array( 'rev_id' => intval( $id ) ) ); + return self::loadFromConds( $db, array( 'rev_id' => intval( $id ) ) ); } /** @@ -195,7 +213,7 @@ class Revision { } else { $conds[] = 'rev_id=page_latest'; } - return Revision::loadFromConds( $db, $conds ); + return self::loadFromConds( $db, $conds ); } /** @@ -214,7 +232,7 @@ class Revision { } else { $matchId = 'page_latest'; } - return Revision::loadFromConds( $db, + return self::loadFromConds( $db, array( "rev_id=$matchId", 'page_namespace' => $title->getNamespace(), 'page_title' => $title->getDBkey() ) @@ -232,7 +250,7 @@ class Revision { * @return Revision or null */ public static function loadFromTimestamp( $db, $title, $timestamp ) { - return Revision::loadFromConds( $db, + return self::loadFromConds( $db, array( 'rev_timestamp' => $db->timestamp( $timestamp ), 'page_namespace' => $title->getNamespace(), 'page_title' => $title->getDBkey() ) @@ -243,14 +261,17 @@ class Revision { * Given a set of conditions, fetch a revision. * * @param $conditions Array + * @param $flags integer (optional) * @return Revision or null */ - public static function newFromConds( $conditions ) { - $db = wfGetDB( DB_SLAVE ); - $rev = Revision::loadFromConds( $db, $conditions ); - if( is_null( $rev ) && wfGetLB()->getServerCount() > 1 ) { - $dbw = wfGetDB( DB_MASTER ); - $rev = Revision::loadFromConds( $dbw, $conditions ); + private static function newFromConds( $conditions, $flags = 0 ) { + $db = wfGetDB( ( $flags & self::LATEST_READ ) ? DB_MASTER : DB_SLAVE ); + $rev = self::loadFromConds( $db, $conditions, $flags ); + if ( is_null( $rev ) && wfGetLB()->getServerCount() > 1 ) { + if ( !( $flags & self::LATEST_READ ) && !( $flags & self::AVOID_MASTER ) ) { + $dbw = wfGetDB( DB_MASTER ); + $rev = self::loadFromConds( $dbw, $conditions, $flags ); + } } return $rev; } @@ -261,10 +282,11 @@ class Revision { * * @param $db DatabaseBase * @param $conditions Array + * @param $flags integer (optional) * @return Revision or null */ - private static function loadFromConds( $db, $conditions ) { - $res = Revision::fetchFromConds( $db, $conditions ); + private static function loadFromConds( $db, $conditions, $flags = 0 ) { + $res = self::fetchFromConds( $db, $conditions, $flags ); if( $res ) { $row = $res->fetchObject(); if( $row ) { @@ -285,7 +307,7 @@ class Revision { * @return ResultWrapper */ public static function fetchRevision( $title ) { - return Revision::fetchFromConds( + return self::fetchFromConds( wfGetDB( DB_SLAVE ), array( 'rev_id=page_latest', 'page_namespace' => $title->getNamespace(), @@ -300,20 +322,25 @@ class Revision { * * @param $db DatabaseBase * @param $conditions Array + * @param $flags integer (optional) * @return ResultWrapper */ - private static function fetchFromConds( $db, $conditions ) { + private static function fetchFromConds( $db, $conditions, $flags = 0 ) { $fields = array_merge( self::selectFields(), self::selectPageFields(), self::selectUserFields() ); + $options = array( 'LIMIT' => 1 ); + if ( $flags & self::FOR_UPDATE ) { + $options[] = 'FOR UPDATE'; + } return $db->select( array( 'revision', 'page', 'user' ), $fields, $conditions, __METHOD__, - array( 'LIMIT' => 1 ), + $options, array( 'page' => self::pageJoinCond(), 'user' => self::userJoinCond() ) ); } @@ -805,7 +832,7 @@ class Revision { if( $this->getTitle() ) { $prev = $this->getTitle()->getPreviousRevisionID( $this->getId() ); if( $prev ) { - return Revision::newFromTitle( $this->getTitle(), $prev ); + return self::newFromTitle( $this->getTitle(), $prev ); } } return null; @@ -820,7 +847,7 @@ class Revision { if( $this->getTitle() ) { $next = $this->getTitle()->getNextRevisionID( $this->getId() ); if ( $next ) { - return Revision::newFromTitle( $this->getTitle(), $next ); + return self::newFromTitle( $this->getTitle(), $next ); } } return null; @@ -950,7 +977,7 @@ class Revision { $text = gzdeflate( $text ); $flags[] = 'gzip'; } else { - wfDebug( "Revision::compressRevisionText() -- no zlib support, not compressing\n" ); + wfDebug( __METHOD__ . " -- no zlib support, not compressing\n" ); } } return implode( ',', $flags ); @@ -969,7 +996,7 @@ class Revision { wfProfileIn( __METHOD__ ); $data = $this->mText; - $flags = Revision::compressRevisionText( $data ); + $flags = self::compressRevisionText( $data ); # Write to external storage if required if( $wgDefaultExternalStore ) { @@ -1019,7 +1046,7 @@ class Revision { ? $this->getPreviousRevisionId( $dbw ) : $this->mParentId, 'rev_sha1' => is_null( $this->mSha1 ) - ? Revision::base36Sha1( $this->mText ) + ? self::base36Sha1( $this->mText ) : $this->mSha1 ), __METHOD__ ); @@ -1241,7 +1268,7 @@ class Revision { static function countByTitle( $db, $title ) { $id = $title->getArticleID(); if( $id ) { - return Revision::countByPageId( $db, $id ); + return self::countByPageId( $db, $id ); } return 0; } diff --git a/includes/dao/IDBAccessObject.php b/includes/dao/IDBAccessObject.php new file mode 100644 index 0000000000..cd5dda92da --- /dev/null +++ b/includes/dao/IDBAccessObject.php @@ -0,0 +1,32 @@ +mOldRev === false || ( $this->mOldRev && $this->mNewRev && $this->mOldRev->getID() == $this->mNewRev->getID() ) ) { @@ -1019,7 +1019,7 @@ class DifferenceEngine extends ContextSource { // Load the new revision object $this->mNewRev = $this->mNewid ? Revision::newFromId( $this->mNewid ) - : Revision::newFromTitle( $this->getTitle() ); + : Revision::newFromTitle( $this->getTitle(), false, Revision::AVOID_MASTER ); if ( !$this->mNewRev instanceof Revision ) { return false; diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 3e95b057ac..1ba57a3bab 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -1455,7 +1455,7 @@ class LocalFile extends File { */ function getDescriptionText() { global $wgParser; - $revision = Revision::newFromTitle( $this->title ); + $revision = Revision::newFromTitle( $this->title, false, Revision::AVOID_MASTER ); if ( !$revision ) return false; $text = $revision->getText(); if ( !$text ) return false; diff --git a/includes/search/SearchEngine.php b/includes/search/SearchEngine.php index 678c530583..a8e04374ff 100644 --- a/includes/search/SearchEngine.php +++ b/includes/search/SearchEngine.php @@ -756,7 +756,8 @@ class SearchResult { protected function initFromTitle( $title ) { $this->mTitle = $title; if ( !is_null( $this->mTitle ) ) { - $this->mRevision = Revision::newFromTitle( $this->mTitle ); + $this->mRevision = Revision::newFromTitle( + $this->mTitle, false, Revision::AVOID_MASTER ); if ( $this->mTitle->getNamespace() === NS_FILE ) $this->mImage = wfFindFile( $this->mTitle ); } diff --git a/includes/specials/SpecialBooksources.php b/includes/specials/SpecialBooksources.php index bc07d586d4..87ba0cb1a6 100644 --- a/includes/specials/SpecialBooksources.php +++ b/includes/specials/SpecialBooksources.php @@ -143,7 +143,7 @@ class SpecialBookSources extends SpecialPage { $page = $this->msg( 'booksources' )->inContentLanguage()->text(); $title = Title::makeTitleSafe( NS_PROJECT, $page ); # Show list in content language if( is_object( $title ) && $title->exists() ) { - $rev = Revision::newFromTitle( $title ); + $rev = Revision::newFromTitle( $title, false, Revision::AVOID_MASTER ); $this->getOutput()->addWikiText( str_replace( 'MAGICNUMBER', $this->isbn, $rev->getText() ) ); return true; } -- 2.20.1