From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 14 Feb 2018 23:23:42 +0000 (-0800)
Subject: Salvage site_stats row with negative values in miser mode
X-Git-Tag: 1.31.0-rc.0~594^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=6535091de23982b0a31642005d47f13b8e554b66

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 7b2b8d38e2..745c89166b 100644
--- 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 );
 
-		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 ) );
 		}
 
-		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.
@@ -79,35 +79,22 @@ class SiteStats {
 			$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
-			$row = (object)array_fill_keys( self::selectFields(), 0 );
+			$row = self::salvageInsaneRow( $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 self::$row->ss_total_edits;
+		return (int)self::$row->ss_total_edits;
 	}
 
 	/**
@@ -116,7 +103,7 @@ class SiteStats {
 	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();
 
-		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();
 
-		return self::$row->ss_users;
+		return (int)self::$row->ss_users;
 	}
 
 	/**
@@ -143,7 +130,7 @@ class SiteStats {
 	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();
 
-		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?
 	 *
@@ -253,7 +253,7 @@ class SiteStats {
 	 * @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
@@ -268,7 +268,7 @@ class SiteStats {
 			'ss_users',
 			'ss_images',
 		] as $member ) {
-			if ( $row->$member > 2000000000 || $row->$member < 0 ) {
+			if ( $row->$member < 0 ) {
 				return false;
 			}
 		}
@@ -276,6 +276,22 @@ class SiteStats {
 		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
 	 */
diff --git a/includes/SiteStatsInit.php b/includes/SiteStatsInit.php
index f8886909a9..f527cb226b 100644
--- a/includes/SiteStatsInit.php
+++ b/includes/SiteStatsInit.php
@@ -198,9 +198,12 @@ class SiteStatsInit {
 
 	/**
 	 * @param int $index
+	 * @param string[] $groups
 	 * @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 );
 	}
 }