jenkins-bot has submitted this change and it was merged. (
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 WAN cache and touch the check keys for each category
whenever those have errors added or deleted from them.
If this happens to get out of sync, it will get fully refreshed
regularly when the totals are sent to statsd.
WANObjectCache's 'lockTSE' feature will help avoid cache stampedes that
made this query expensive in the past.
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, 171 insertions(+), 17 deletions(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index b2dc946..5178a92 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..7063169 100644
--- a/includes/RecordLintJob.php
+++ b/includes/RecordLintJob.php
@@ -62,22 +62,46 @@
$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;
+ $mwServices = MediaWikiServices::getInstance();
+
+ $totalsLookup = new TotalsLookup(
+ new CategoryManager(),
+ $mwServices->getMainWANObjectCache()
+ );
+
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 ) {
+ if ( $count != 0 ) {
+ // There was a change in counts,
invalidate the cache
+ $totalsLookup->touchCategoryCache( $cat
);
+ }
+ }
return;
} elseif ( mt_rand( 1, $wgLinterStatsdSampleFactor ) != 1 ) {
return;
@@ -86,12 +110,14 @@
$totals = $lintDb->getTotals();
$wiki = wfWikiID();
- $stats =
MediaWikiServices::getInstance()->getStatsdDataFactory();
+ $stats = $mwServices->getStatsdDataFactory();
foreach ( $totals as $name => $count ) {
$stats->gauge( "linter.category.$name.$wiki", $count );
}
$stats->gauge( "linter.totals.$wiki", array_sum( $totals ) );
+
+ $totalsLookup->touchAllCategoriesCache();
}
}
diff --git a/includes/SpecialLintErrors.php b/includes/SpecialLintErrors.php
index 7b1a105..3ae7e7a 100644
--- a/includes/SpecialLintErrors.php
+++ b/includes/SpecialLintErrors.php
@@ -22,6 +22,8 @@
use HTMLForm;
use Html;
+use MediaWiki\MediaWikiServices;
+use ObjectCache;
use SpecialPage;
class SpecialLintErrors extends SpecialPage {
@@ -94,7 +96,11 @@
}
private function showCategoryListings( CategoryManager $catManager ) {
- $totals = ( new Database( 0 ) )->getTotals();
+ $lookup = new TotalsLookup(
+ $catManager,
+
MediaWikiServices::getInstance()->getMainWANObjectCache()
+ );
+ $totals = $lookup->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..1830855
--- /dev/null
+++ b/includes/TotalsLookup.php
@@ -0,0 +1,104 @@
+<?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 WANObjectCache;
+use Wikimedia\Rdbms\Database as MWDatabase;
+
+/**
+ * Lookup to find and cache the total amount of
+ * lint errors in each category
+ */
+class TotalsLookup {
+
+ /**
+ * @var WANObjectCache
+ */
+ private $cache;
+
+ /**
+ * @var CategoryManager
+ */
+ private $catManager;
+
+ public function __construct( CategoryManager $catManager,
WANObjectCache $cache ) {
+ $this->cache = $cache;
+ $this->catManager = $catManager;
+ }
+
+ /**
+ * @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();
+ $fetchedTotals = false;
+ $totals = [];
+ foreach ( $cats as $cat ) {
+ $totals[$cat] = $this->cache->getWithSetCallback(
+ $this->makeKey( $cat ),
+ WANObjectCache::TTL_INDEFINITE,
+ function( $oldValue, &$ttl, &$setOpts, $oldAsOf
) use ( $cat, &$fetchedTotals ) {
+ $setOpts +=
MWDatabase::getCacheSetOptions( wfGetDB( DB_REPLICA ) );
+ if ( $fetchedTotals === false ) {
+ $fetchedTotals = ( new
Database( 0 ) )->getTotals();
+ }
+
+ return $fetchedTotals[$cat];
+ },
+ [
+ 'checkKeys' => [
+ $this->cache->makeKey(
'linter', 'totals' ),
+ $this->makeKey( $cat ),
+ ],
+ 'lockTSE' => 30,
+ ]
+ );
+ }
+
+ return $totals;
+ }
+
+ /**
+ * Have a single category be recalculated
+ *
+ * @param string $cat category name
+ */
+ public function touchCategoryCache( $cat ) {
+ $this->cache->touchCheckKey( $this->makeKey( $cat ) );
+ }
+
+ /**
+ * Have all categories be recalculated
+ */
+ public function touchAllCategoriesCache() {
+ $this->cache->touchCheckKey( $this->cache->makeKey( 'linter',
'totals' ) );
+ }
+}
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: merged
Gerrit-Change-Id: I3774103a29fa0f29d36283950f136259fa71bffe
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/Linter
Gerrit-Branch: master
Gerrit-Owner: Legoktm <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Arlolra <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Subramanya Sastry <[email protected]>
Gerrit-Reviewer: Tim Starling <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
