Legoktm has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/350958 )
Change subject: Add caching to looking up totals
......................................................................
Add caching to looking up totals
The query itself is too expensive to be run on large Wikimedia wikis. So
put it behind memcache and increment/decrement the count for each
category as we add and delete errors from the database.
If this happens to get out of sync, it will get fully refreshed
regularly when the totals are sent to statsd.
Change-Id: I3774103a29fa0f29d36283950f136259fa71bffe
---
M extension.json
M includes/Database.php
M includes/RecordLintJob.php
M includes/SpecialLintErrors.php
A includes/TotalsLookup.php
M tests/phpunit/DatabaseTest.php
6 files changed, 162 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Linter
refs/changes/58/350958/1
diff --git a/extension.json b/extension.json
index 31af050..7d3164a 100644
--- a/extension.json
+++ b/extension.json
@@ -16,7 +16,8 @@
"MediaWiki\\Linter\\ApiQueryLintErrors":
"includes/ApiQueryLintErrors.php",
"MediaWiki\\Linter\\RecordLintJob":
"includes/RecordLintJob.php",
"MediaWiki\\Linter\\SpecialLintErrors":
"includes/SpecialLintErrors.php",
- "MediaWiki\\Linter\\LintErrorsPager":
"includes/LintErrorsPager.php"
+ "MediaWiki\\Linter\\LintErrorsPager":
"includes/LintErrorsPager.php",
+ "MediaWiki\\Linter\\TotalsLookup": "includes/TotalsLookup.php"
},
"MessagesDirs": {
"Linter": "i18n"
diff --git a/includes/Database.php b/includes/Database.php
index 471b8d7..d6f93b1 100644
--- a/includes/Database.php
+++ b/includes/Database.php
@@ -130,17 +130,34 @@
}
/**
+ * @param LintError[] $errors
+ * @return array
+ */
+ private function countByCat( array $errors ) {
+ $count = [];
+ foreach ( $errors as $error ) {
+ if ( !isset( $count[$error->category] ) ) {
+ $count[$error->category] = 1;
+ } else {
+ $count[$error->category] += 1;
+ }
+ }
+
+ return $count;
+ }
+
+ /**
* Save the specified lint errors in the
* database
*
* @param LintError[] $errors
- * @return array [ 'deleted' => int, 'added' => int ]
+ * @return array [ 'deleted' => [ cat => count ], 'added' => [ cat =>
count ] ]
*/
public function setForPage( $errors ) {
$previous = $this->getForPage();
$dbw = wfGetDB( DB_MASTER );
if ( !$previous && !$errors ) {
- return [ 'deleted' => 0, 'added' => 0 ];
+ return [ 'deleted' => [], 'added' => [] ];
} elseif ( !$previous && $errors ) {
$toInsert = array_values( $errors );
$toDelete = [];
@@ -150,7 +167,7 @@
[ 'linter_page' => $this->pageId ],
__METHOD__
);
- return [ 'deleted' => count( $previous ), 'added' => 0
];
+ return [ 'deleted' => $this->countByCat( $previous ),
'added' => [] ];
} else {
$toInsert = [];
$toDelete = $previous;
@@ -191,8 +208,8 @@
}
return [
- 'deleted' => count( $toDelete ),
- 'added' => count( $toInsert ),
+ 'deleted' => $this->countByCat( $toDelete ),
+ 'added' => $this->countByCat( $toInsert ),
];
}
diff --git a/includes/RecordLintJob.php b/includes/RecordLintJob.php
index a560ec0..ad00c99 100644
--- a/includes/RecordLintJob.php
+++ b/includes/RecordLintJob.php
@@ -62,22 +62,38 @@
$toSet = array_merge( $toSet, $catErrors );
}
- $lintDb->setForPage( $toSet );
- $this->updateStats( $lintDb );
+ $changes = $lintDb->setForPage( $toSet );
+ $this->updateStats( $lintDb, $changes );
return true;
}
/**
- * Send stats to statsd
+ * Send stats to statsd and update totals cache
*
* @param Database $lintDb
+ * @param array $changes
*/
- protected function updateStats( Database $lintDb ) {
+ protected function updateStats( Database $lintDb, array $changes ) {
global $wgLinterStatsdSampleFactor;
+ $totalsLookup = new TotalsLookup();
+
if ( $wgLinterStatsdSampleFactor === false ) {
- // Not enabled at all
+ // Don't send to statsd, but update cache with $changes
+ $raw = $changes['added'];
+ foreach ( $changes['deleted'] as $cat => $count ) {
+ if ( isset( $raw[$cat] ) ) {
+ $raw[$cat] -= $count;
+ } else {
+ // Negative value
+ $raw[$cat] = 0 - $count;
+ }
+ }
+
+ foreach ( $raw as $cat => $count ) {
+ $totalsLookup->incr( $cat, $count );
+ }
return;
} elseif ( mt_rand( 1, $wgLinterStatsdSampleFactor ) != 1 ) {
return;
@@ -92,6 +108,8 @@
}
$stats->gauge( "linter.totals.$wiki", array_sum( $totals ) );
+
+ $totalsLookup->setTotals( $totals );
}
}
diff --git a/includes/SpecialLintErrors.php b/includes/SpecialLintErrors.php
index 7b1a105..500ee8b 100644
--- a/includes/SpecialLintErrors.php
+++ b/includes/SpecialLintErrors.php
@@ -94,7 +94,7 @@
}
private function showCategoryListings( CategoryManager $catManager ) {
- $totals = ( new Database( 0 ) )->getTotals();
+ $totals = ( new TotalsLookup() )->getTotals();
// Display lint issues by priority
$this->displayList( 'high', $totals,
$catManager->getHighPriority() );
diff --git a/includes/TotalsLookup.php b/includes/TotalsLookup.php
new file mode 100644
index 0000000..c7222fd
--- /dev/null
+++ b/includes/TotalsLookup.php
@@ -0,0 +1,110 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Linter;
+
+use BagOStuff;
+use ObjectCache;
+
+/**
+ * Lookup to find and cache the total amount of
+ * lint errors in each category
+ */
+class TotalsLookup {
+
+ /**
+ * @var ObjectCache
+ */
+ private $cache;
+
+ /**
+ * @var CategoryManager
+ */
+ private $catManager;
+
+ public function __construct() {
+ $this->cache = ObjectCache::getLocalClusterInstance();
+ $this->catManager = new CategoryManager();
+ }
+
+ /**
+ * @param string $cat
+ * @return string
+ */
+ private function makeKey( $cat ) {
+ return $this->cache->makeKey( 'linter', 'total', $cat );
+ }
+
+ /**
+ * Get the totals for every category in the database
+ *
+ * @return array
+ */
+ public function getTotals() {
+ $cats = $this->catManager->getVisibleCategories();
+ $totals = [];
+ $fetchedTotals = false;
+ foreach ( $cats as $cat ) {
+ $totals[$cat] = $this->cache->getWithSetCallback(
+ $this->makeKey( $cat ),
+ BagOStuff::TTL_HOUR,
+ function() use ( &$fetchedTotals, $cat ) {
+ if ( $fetchedTotals === false ) {
+ // Only do this query once
+ $fetchedTotals = ( new
Database( 0 ) )->getTotals();
+ }
+
+ return $fetchedTotals[$cat];
+ }
+ );
+ }
+
+ return $totals;
+ }
+
+ /**
+ * If we had to calculate the totals some other way,
+ * update the cache too
+ *
+ * @param array $totals
+ */
+ public function setTotals( array $totals ) {
+ $toSet = [];
+ foreach ( $totals as $cat => $val ) {
+ $toSet[$this->makeKey( $cat )] = $val;
+ }
+
+ $this->cache->setMulti( $toSet, BagOStuff::TTL_HOUR );
+ }
+
+ /**
+ * Update the count for a category based on how many errors changed
+ *
+ * @param string $cat
+ * @param int $count
+ */
+ public function incr( $cat, $count ) {
+ if ( $count > 0 ) {
+ $this->cache->incr( $this->makeKey( $cat ), $count );
+ } elseif ( $count < 0 ) {
+ $this->cache->decr( $this->makeKey( $cat ), abs( $count
) );
+ }
+ }
+}
diff --git a/tests/phpunit/DatabaseTest.php b/tests/phpunit/DatabaseTest.php
index 22215ac..c8133d4 100644
--- a/tests/phpunit/DatabaseTest.php
+++ b/tests/phpunit/DatabaseTest.php
@@ -66,22 +66,22 @@
$lintDb = new Database( 5 );
$dummyErrors = $this->getDummyLintErrors();
$result = $lintDb->setForPage( $dummyErrors );
- $this->assertSetForPageResult( $result, 0, 2 );
+ $this->assertSetForPageResult( $result, [], [ 'fostered' => 1,
'obsolete-tag' => 1 ] );
$this->assertLintErrorsEqual( $dummyErrors,
$lintDb->getForPage() );
// Should delete the second error
$result2 = $lintDb->setForPage( [ $dummyErrors[0] ] );
- $this->assertSetForPageResult( $result2, 1, 0 );
+ $this->assertSetForPageResult( $result2, [ 'obsolete-tag' => 1
], [] );
$this->assertLintErrorsEqual( [ $dummyErrors[0] ],
$lintDb->getForPage() );
// Insert the second error, delete the first
$result3 = $lintDb->setForPage( [ $dummyErrors[1] ] );
- $this->assertSetForPageResult( $result3, 1, 1 );
+ $this->assertSetForPageResult( $result3, [ 'fostered' => 1 ], [
'obsolete-tag' => 1 ] );
$this->assertLintErrorsEqual( [ $dummyErrors[1] ],
$lintDb->getForPage() );
// Delete the second (only) error
$result4 = $lintDb->setForPage( [] );
- $this->assertSetForPageResult( $result4, 1, 0 );
+ $this->assertSetForPageResult( $result4, [ 'obsolete-tag' => 1
], [] );
$this->assertLintErrorsEqual( [], $lintDb->getForPage() );
}
--
To view, visit https://gerrit.wikimedia.org/r/350958
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3774103a29fa0f29d36283950f136259fa71bffe
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Linter
Gerrit-Branch: master
Gerrit-Owner: Legoktm <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
