Salvage site_stats row with negative values in miser mode

* Instead of returning all zeroes, just use zero for the
  negative values in the row.
* Allow large numbers since the fields are BIGINT.
* Clean up the return types to truly be integers.
* Respect the $groups argument in SiteStatsInit::getDB().

Bug: T186947
Change-Id: I51fdc45124c12aba114540fc0ec66a3e63d61e09

diff --git a/includes/SiteStats.php b/includes/SiteStats.php
index 7b2b8d3..745c891 100644 (file)
--- a/includes/SiteStats.php
+++ b/includes/SiteStats.php
@@ -56,13 +56,13 @@ class SiteStats {
                wfDebug( __METHOD__ . ": reading site_stats from replica DB\n" );
                $row = self::doLoadFromDB( $dbr );
 
                wfDebug( __METHOD__ . ": reading site_stats from replica DB\n" );
                $row = self::doLoadFromDB( $dbr );
 

-               if ( !self::isSane( $row ) && $lb->hasOrMadeRecentMasterChanges() ) {
+               if ( !self::isRowSane( $row ) && $lb->hasOrMadeRecentMasterChanges() ) {

                        // Might have just been initialized during this request? Underflow?
                        wfDebug( __METHOD__ . ": site_stats damaged or missing on replica DB\n" );
                        $row = self::doLoadFromDB( $lb->getConnection( DB_MASTER ) );
                }
 
                        // Might have just been initialized during this request? Underflow?
                        wfDebug( __METHOD__ . ": site_stats damaged or missing on replica DB\n" );
                        $row = self::doLoadFromDB( $lb->getConnection( DB_MASTER ) );
                }
 

-               if ( !self::isSane( $row ) ) {
+               if ( !self::isRowSane( $row ) ) {

                        if ( $config->get( 'MiserMode' ) ) {
                                // Start off with all zeroes, assuming that this is a new wiki or any
                                // repopulations where done manually via script.
                        if ( $config->get( 'MiserMode' ) ) {
                                // Start off with all zeroes, assuming that this is a new wiki or any
                                // repopulations where done manually via script.


@@ -79,35 +79,22 @@ class SiteStats {
                        $row = self::doLoadFromDB( $lb->getConnection( DB_MASTER ) );
                }
 
                        $row = self::doLoadFromDB( $lb->getConnection( DB_MASTER ) );
                }
 

-               if ( !self::isSane( $row ) ) {
+               if ( !self::isRowSane( $row ) ) {

                        wfDebug( __METHOD__ . ": site_stats persistently nonsensical o_O\n" );
                        // Always return a row-like object
                        wfDebug( __METHOD__ . ": site_stats persistently nonsensical o_O\n" );
                        // Always return a row-like object

-                       $row = (object)array_fill_keys( self::selectFields(), 0 );
+                       $row = self::salvageInsaneRow( $row );

                }
 
                return $row;
        }
 
                }
 
                return $row;
        }
 

-       /**
-        * @param IDatabase $db
-        * @return stdClass|bool
-        */
-       private static function doLoadFromDB( IDatabase $db ) {
-               return $db->selectRow(
-                       'site_stats',
-                       self::selectFields(),
-                       [ 'ss_row_id' => 1 ],
-                       __METHOD__
-               );
-       }
-

        /**
         * @return int
         */
        public static function edits() {
                self::load();
 
        /**
         * @return int
         */
        public static function edits() {
                self::load();
 

-               return self::$row->ss_total_edits;
+               return (int)self::$row->ss_total_edits;

        }
 
        /**
        }
 
        /**


@@ -116,7 +103,7 @@ class SiteStats {
        public static function articles() {
                self::load();
 
        public static function articles() {
                self::load();
 

-               return self::$row->ss_good_articles;
+               return (int)self::$row->ss_good_articles;

        }
 
        /**
        }
 
        /**


@@ -125,7 +112,7 @@ class SiteStats {
        public static function pages() {
                self::load();
 
        public static function pages() {
                self::load();
 

-               return self::$row->ss_total_pages;
+               return (int)self::$row->ss_total_pages;

        }
 
        /**
        }
 
        /**


@@ -134,7 +121,7 @@ class SiteStats {
        public static function users() {
                self::load();
 
        public static function users() {
                self::load();
 

-               return self::$row->ss_users;
+               return (int)self::$row->ss_users;

        }
 
        /**
        }
 
        /**


@@ -143,7 +130,7 @@ class SiteStats {
        public static function activeUsers() {
                self::load();
 
        public static function activeUsers() {
                self::load();
 

-               return self::$row->ss_active_users;
+               return (int)self::$row->ss_active_users;

        }
 
        /**
        }
 
        /**


@@ -152,7 +139,7 @@ class SiteStats {
        public static function images() {
                self::load();
 
        public static function images() {
                self::load();
 

-               return self::$row->ss_images;
+               return (int)self::$row->ss_images;

        }
 
        /**
        }
 
        /**


@@ -245,6 +232,19 @@ class SiteStats {
                ];
        }
 
                ];
        }
 

+       /**
+        * @param IDatabase $db
+        * @return stdClass|bool
+        */
+       private static function doLoadFromDB( IDatabase $db ) {
+               return $db->selectRow(
+                       'site_stats',
+                       self::selectFields(),
+                       [ 'ss_row_id' => 1 ],
+                       __METHOD__
+               );
+       }
+

        /**
         * Is the provided row of site stats sane, or should it be regenerated?
         *
        /**
         * Is the provided row of site stats sane, or should it be regenerated?
         *


@@ -253,7 +253,7 @@ class SiteStats {
         * @param bool|object $row
         * @return bool
         */
         * @param bool|object $row
         * @return bool
         */

-       private static function isSane( $row ) {
+       private static function isRowSane( $row ) {

                if ( $row === false
                        || $row->ss_total_pages < $row->ss_good_articles
                        || $row->ss_total_edits < $row->ss_total_pages
                if ( $row === false
                        || $row->ss_total_pages < $row->ss_good_articles
                        || $row->ss_total_edits < $row->ss_total_pages


@@ -268,7 +268,7 @@ class SiteStats {
                        'ss_users',
                        'ss_images',
                ] as $member ) {
                        'ss_users',
                        'ss_images',
                ] as $member ) {

-                       if ( $row->$member > 2000000000 || $row->$member < 0 ) {
+                       if ( $row->$member < 0 ) {

                                return false;
                        }
                }
                                return false;
                        }
                }


@@ -276,6 +276,22 @@ class SiteStats {
                return true;
        }
 
                return true;
        }
 

+       /**
+        * @param stdClass|bool $row
+        * @return stdClass
+        */
+       private static function salvageInsaneRow( $row ) {
+               $map = $row ? (array)$row : [];
+               // Fill in any missing values with zero
+               $map += array_fill_keys( self::selectFields(), 0 );
+               // Convert negative values to zero
+               foreach ( $map as $field => $value ) {
+                       $map[$field] = max( 0, $value );
+               }
+
+               return (object)$row;
+       }
+

        /**
         * @return LoadBalancer
         */
        /**
         * @return LoadBalancer
         */

diff --git a/includes/SiteStatsInit.php b/includes/SiteStatsInit.php
index f888690..f527cb2 100644 (file)
--- a/includes/SiteStatsInit.php
+++ b/includes/SiteStatsInit.php
@@ -198,9 +198,12 @@ class SiteStatsInit {
 
        /**
         * @param int $index
 
        /**
         * @param int $index

+        * @param string[] $groups

         * @return IDatabase
         */
         * @return IDatabase
         */

-       private static function getDB( $index ) {
-               return MediaWikiServices::getInstance()->getDBLoadBalancer()->getConnection( $index );
+       private static function getDB( $index, $groups = [] ) {
+               $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
+
+               return $lb->getConnection( $index, $groups );

        }
 }
        }
 }