From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 2 Jul 2018 11:19:23 +0000 (+0100)
Subject: rdbms: add "maxReadRows" limit to TransactionProfiler
X-Git-Tag: 1.34.0-rc.0~4461^2
X-Git-Url: https://git.heureux-cyclage.org/?a=commitdiff_plain;h=f3a197e49b7851fbfdd6c464f922ccac8895cdd7;hp=6129d1d704a9833b23c92a371fe54475323cc190;p=lhc%2Fweb%2Fwiklou.git

rdbms: add "maxReadRows" limit to TransactionProfiler

Change-Id: I008fc1857c7aa43d93f43361803d5e52c4ddf089
---

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 27769f973d..0ed476e799 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -6189,24 +6189,28 @@ $wgTrxProfilerLimits = [
 	'GET' => [
 		'masterConns' => 0,
 		'writes' => 0,
-		'readQueryTime' => 5
+		'readQueryTime' => 5,
+		'readQueryRows' => 10000
 	],
 	// HTTP POST requests.
 	// Master reads and writes will happen for a subset of these.
 	'POST' => [
 		'readQueryTime' => 5,
 		'writeQueryTime' => 1,
+		'readQueryRows' => 100000,
 		'maxAffected' => 1000
 	],
 	'POST-nonwrite' => [
 		'masterConns' => 0,
 		'writes' => 0,
-		'readQueryTime' => 5
+		'readQueryTime' => 5,
+		'readQueryRows' => 10000
 	],
 	// Deferred updates that run after HTTP response is sent for GET requests
 	'PostSend-GET' => [
 		'readQueryTime' => 5,
 		'writeQueryTime' => 1,
+		'readQueryRows' => 10000,
 		'maxAffected' => 1000,
 		// Log master queries under the post-send entry point as they are discouraged
 		'masterConns' => 0,
@@ -6216,12 +6220,14 @@ $wgTrxProfilerLimits = [
 	'PostSend-POST' => [
 		'readQueryTime' => 5,
 		'writeQueryTime' => 1,
+		'readQueryRows' => 100000,
 		'maxAffected' => 1000
 	],
 	// Background job runner
 	'JobRunner' => [
 		'readQueryTime' => 30,
 		'writeQueryTime' => 5,
+		'readQueryRows' => 100000,
 		'maxAffected' => 500 // ballpark of $wgUpdateRowsPerQuery
 	],
 	// Command-line scripts
diff --git a/includes/libs/rdbms/TransactionProfiler.php b/includes/libs/rdbms/TransactionProfiler.php
index c353a224c6..8ea28f0a65 100644
--- a/includes/libs/rdbms/TransactionProfiler.php
+++ b/includes/libs/rdbms/TransactionProfiler.php
@@ -62,6 +62,7 @@ class TransactionProfiler implements LoggerAwareInterface {
 		'conns'          => INF,
 		'masterConns'    => INF,
 		'maxAffected'    => INF,
+		'readQueryRows'  => INF,
 		'readQueryTime'  => INF,
 		'writeQueryTime' => INF
 	];
@@ -199,7 +200,7 @@ class TransactionProfiler implements LoggerAwareInterface {
 	 * @param string $query Function name or generalized SQL
 	 * @param float $sTime Starting UNIX wall time
 	 * @param bool $isWrite Whether this is a write query
-	 * @param int $n Number of affected rows
+	 * @param int $n Number of affected/read rows
 	 */
 	public function recordQueryCompletion( $query, $sTime, $isWrite = false, $n = 0 ) {
 		$eTime = microtime( true );
@@ -209,6 +210,10 @@ class TransactionProfiler implements LoggerAwareInterface {
 			$this->logger->warning(
 				"Query affected $n row(s):\n" . $query . "\n" .
 				( new RuntimeException() )->getTraceAsString() );
+		} elseif ( !$isWrite && $n > $this->expect['readQueryRows'] ) {
+			$this->logger->warning(
+				"Query returned $n row(s):\n" . $query . "\n" .
+				( new RuntimeException() )->getTraceAsString() );
 		}
 
 		// Report when too many writes/queries happen...
diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 4070a02b32..2673b2c001 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1239,7 +1239,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 		}
 
 		$this->trxProfiler->recordQueryCompletion(
-			$queryProf, $startTime, $isWrite, $this->affectedRows()
+			$queryProf,
+			$startTime,
+			$isWrite,
+			$isWrite ? $this->affectedRows() : $this->numRows( $ret )
 		);
 		$this->queryLogger->debug( $sql, [
 			'method' => $fname,
diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php
index a043cd6a89..b8a92004ce 100644
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -299,6 +299,10 @@ class DatabasePostgres extends Database {
 	}
 
 	public function numRows( $res ) {
+		if ( $res === false ) {
+			return 0;
+		}
+
 		if ( $res instanceof ResultWrapper ) {
 			$res = $res->result;
 		}
diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index b0fd12a69c..25fbba09e5 100644
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -399,13 +399,14 @@ class DatabaseSqlite extends Database {
 	/**
 	 * The PDO::Statement class implements the array interface so count() will work
 	 *
-	 * @param ResultWrapper|array $res
+	 * @param ResultWrapper|array|false $res
 	 * @return int
 	 */
 	function numRows( $res ) {
+		// false does not implement Countable
 		$r = $res instanceof ResultWrapper ? $res->result : $res;
 
-		return count( $r );
+		return is_array( $r ) ? count( $r ) : 0;
 	}
 
 	/**