Added limit to countRevisionsBetween() for sanity
authorAaron Schulz <aschulz@wikimedia.org>
Mon, 7 Apr 2014 22:18:19 +0000 (15:18 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 7 Apr 2014 22:18:19 +0000 (15:18 -0700)
* This was causing dozens of DB query timeout errors at WMF

Change-Id: Ie51a491ff5d22c2d84934e83d4b3f690c9dbd595

includes/Title.php
includes/diff/DifferenceEngine.php

index d025f35..34ebf3a 100644 (file)
@@ -4343,7 +4343,7 @@ class Title {
         * @param int|Revision $new New revision or rev ID (first after range)
         * @return Int Number of revisions between these revisions.
         */
-       public function countRevisionsBetween( $old, $new ) {
+       public function countRevisionsBetween( $old, $new, $max = null ) {
                if ( !( $old instanceof Revision ) ) {
                        $old = Revision::newFromTitle( $this, (int)$old );
                }
@@ -4354,14 +4354,21 @@ class Title {
                        return 0; // nothing to compare
                }
                $dbr = wfGetDB( DB_SLAVE );
-               return (int)$dbr->selectField( 'revision', 'count(*)',
-                       array(
-                               'rev_page' => $this->getArticleID(),
-                               'rev_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ),
-                               'rev_timestamp < ' . $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) )
-                       ),
-                       __METHOD__
+               $conds = array(
+                       'rev_page' => $this->getArticleID(),
+                       'rev_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( $old->getTimestamp() ) ),
+                       'rev_timestamp < ' . $dbr->addQuotes( $dbr->timestamp( $new->getTimestamp() ) )
                );
+               if ( $max !== null ) {
+                       $res = $dbr->select( 'revision', '1',
+                               $conds,
+                               __METHOD__,
+                               array( 'LIMIT' => $max + 1 ) // extra to detect truncation
+                       );
+                       return $res->numRows();
+               } else {
+                       return (int)$dbr->selectField( 'revision', 'count(*)', $conds, __METHOD__ );
+               }
        }
 
        /**
index f77a4eb..3c4773e 100644 (file)
@@ -970,8 +970,10 @@ class DifferenceEngine extends ContextSource {
                        $newRev = $this->mNewRev;
                }
 
-               $nEdits = $this->mNewPage->countRevisionsBetween( $oldRev, $newRev );
-               if ( $nEdits > 0 ) {
+               // Sanity: don't show the notice if too many rows must be scanned
+               // @TODO: show some special message for that case
+               $nEdits = $this->mNewPage->countRevisionsBetween( $oldRev, $newRev, 1000 );
+               if ( $nEdits > 0 && $nEdits <= 1000 ) {
                        $limit = 100; // use diff-multi-manyusers if too many users
                        $users = $this->mNewPage->getAuthorsBetween( $oldRev, $newRev, $limit );
                        $numUsers = count( $users );