From: Aaron Schulz Date: Sat, 6 Jul 2019 20:36:43 +0000 (-0700) Subject: Remove $wgSiteStatsAsyncFactor feature and related $wgMainStash use X-Git-Tag: 1.34.0-rc.0~1016 X-Git-Url: http://git.cyclocoop.org/%22.%20generer_url_ecrire%28%22sites_tous%22%2C%22%22%29.%20%22?a=commitdiff_plain;h=149b5a016cfc9fcb8ada9028ae940afb2a36dc40;p=lhc%2Fweb%2Fwiklou.git Remove $wgSiteStatsAsyncFactor feature and related $wgMainStash use Also include ss_row_id = 1 in the UPDATE query to avoid gap locks Bug: T227376 Change-Id: I7b730bab05e6d8b6799b623e0aff089c1103c3c2 --- diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 0886f3860a..49e7a34c70 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -6523,14 +6523,6 @@ $wgStatsdSamplingRates = [ */ $wgPageInfoTransclusionLimit = 50; -/** - * Set this to an integer to only do synchronous site_stats updates - * one every *this many* updates. The other requests go into pending - * delta values in $wgMemc. Make sure that $wgMemc is a global cache. - * If set to -1, updates *only* go to $wgMemc (useful for daemons). - */ -$wgSiteStatsAsyncFactor = false; - /** * Parser test suite files to be run by parserTests.php when no specific * filename is passed to it. diff --git a/includes/deferred/SiteStatsUpdate.php b/includes/deferred/SiteStatsUpdate.php index 007cf0e5e7..11e9337093 100644 --- a/includes/deferred/SiteStatsUpdate.php +++ b/includes/deferred/SiteStatsUpdate.php @@ -25,8 +25,6 @@ use Wikimedia\Rdbms\IDatabase; * Class for handling updates to the site_stats table */ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate { - /** @var BagOStuff */ - protected $stash; /** @var int */ protected $edits = 0; /** @var int */ @@ -38,7 +36,14 @@ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate { /** @var int */ protected $images = 0; - private static $counters = [ 'edits', 'pages', 'articles', 'users', 'images' ]; + /** @var string[] Map of (table column => counter type) */ + private static $counters = [ + 'ss_total_edits' => 'edits', + 'ss_total_pages' => 'pages', + 'ss_good_articles' => 'articles', + 'ss_users' => 'users', + 'ss_images' => 'images' + ]; // @todo deprecate this constructor function __construct( $views, $edits, $good, $pages = 0, $users = 0 ) { @@ -46,8 +51,6 @@ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate { $this->articles = $good; $this->pages = $pages; $this->users = $users; - - $this->stash = MediaWikiServices::getInstance()->getMainObjectStash(); } public function merge( MergeableUpdate $update ) { @@ -60,8 +63,9 @@ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate { } /** - * @param array $deltas + * @param int[] $deltas Map of (counter type => integer delta) * @return SiteStatsUpdate + * @throws UnexpectedValueException */ public static function factory( array $deltas ) { $update = new self( 0, 0, 0 ); @@ -73,73 +77,46 @@ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate { } foreach ( self::$counters as $field ) { - if ( isset( $deltas[$field] ) && $deltas[$field] ) { - $update->$field = $deltas[$field]; - } + $update->$field = $deltas[$field] ?? 0; } return $update; } public function doUpdate() { - $this->doUpdateContextStats(); - - $rate = MediaWikiServices::getInstance()->getMainConfig()->get( 'SiteStatsAsyncFactor' ); - // If set to do so, only do actual DB updates 1 every $rate times. - // The other times, just update "pending delta" values in memcached. - if ( $rate && ( $rate < 0 || mt_rand( 0, $rate - 1 ) != 0 ) ) { - $this->doUpdatePendingDeltas(); - } else { - // Need a separate transaction because this a global lock - DeferredUpdates::addCallableUpdate( [ $this, 'tryDBUpdateInternal' ] ); - } - } - - /** - * Do not call this outside of SiteStatsUpdate - */ - public function tryDBUpdateInternal() { $services = MediaWikiServices::getInstance(); - $config = $services->getMainConfig(); - - $dbw = $services->getDBLoadBalancer()->getConnectionRef( DB_MASTER ); - $lockKey = $dbw->getDomainID() . ':site_stats'; // prepend wiki ID - $pd = []; - if ( $config->get( 'SiteStatsAsyncFactor' ) ) { - // Lock the table so we don't have double DB/memcached updates - if ( !$dbw->lock( $lockKey, __METHOD__, 0 ) ) { - $this->doUpdatePendingDeltas(); + $stats = $services->getStatsdDataFactory(); - return; + $deltaByType = []; + foreach ( self::$counters as $type ) { + $delta = $this->$type; + if ( $delta !== 0 ) { + $stats->updateCount( "site.$type", $delta ); } - $pd = $this->getPendingDeltas(); - // Piggy-back the async deltas onto those of this stats update.... - $this->edits += ( $pd['ss_total_edits']['+'] - $pd['ss_total_edits']['-'] ); - $this->articles += ( $pd['ss_good_articles']['+'] - $pd['ss_good_articles']['-'] ); - $this->pages += ( $pd['ss_total_pages']['+'] - $pd['ss_total_pages']['-'] ); - $this->users += ( $pd['ss_users']['+'] - $pd['ss_users']['-'] ); - $this->images += ( $pd['ss_images']['+'] - $pd['ss_images']['-'] ); - } - - // Build up an SQL query of deltas and apply them... - $updates = ''; - $this->appendUpdate( $updates, 'ss_total_edits', $this->edits ); - $this->appendUpdate( $updates, 'ss_good_articles', $this->articles ); - $this->appendUpdate( $updates, 'ss_total_pages', $this->pages ); - $this->appendUpdate( $updates, 'ss_users', $this->users ); - $this->appendUpdate( $updates, 'ss_images', $this->images ); - if ( $updates != '' ) { - $dbw->update( 'site_stats', [ $updates ], [], __METHOD__ ); + $deltaByType[$type] = $delta; } - if ( $config->get( 'SiteStatsAsyncFactor' ) ) { - // Decrement the async deltas now that we applied them - $this->removePendingDeltas( $pd ); - // Commit the updates and unlock the table - $dbw->unlock( $lockKey, __METHOD__ ); - } + ( new AutoCommitUpdate( + $services->getDBLoadBalancer()->getConnectionRef( DB_MASTER ), + __METHOD__, + function ( IDatabase $dbw, $fname ) use ( $deltaByType ) { + $set = []; + foreach ( self::$counters as $column => $type ) { + $delta = (int)$deltaByType[$type]; + if ( $delta > 0 ) { + $set[] = "$column=$column+" . abs( $delta ); + } elseif ( $delta < 0 ) { + $set[] = "$column=$column-" . abs( $delta ); + } + } + + if ( $set ) { + $dbw->update( 'site_stats', $set, [ 'ss_row_id' => 1 ], $fname ); + } + } + ) )->doUpdate(); - // Invalid cache used by parser functions + // Invalidate cache used by parser functions SiteStats::unload(); } @@ -182,105 +159,4 @@ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate { return $activeUsers; } - - protected function doUpdateContextStats() { - $stats = MediaWikiServices::getInstance()->getStatsdDataFactory(); - foreach ( [ 'edits', 'articles', 'pages', 'users', 'images' ] as $type ) { - $delta = $this->$type; - if ( $delta !== 0 ) { - $stats->updateCount( "site.$type", $delta ); - } - } - } - - protected function doUpdatePendingDeltas() { - $this->adjustPending( 'ss_total_edits', $this->edits ); - $this->adjustPending( 'ss_good_articles', $this->articles ); - $this->adjustPending( 'ss_total_pages', $this->pages ); - $this->adjustPending( 'ss_users', $this->users ); - $this->adjustPending( 'ss_images', $this->images ); - } - - /** - * @param string &$sql - * @param string $field - * @param int $delta - */ - protected function appendUpdate( &$sql, $field, $delta ) { - if ( $delta ) { - if ( $sql ) { - $sql .= ','; - } - if ( $delta < 0 ) { - $sql .= "$field=$field-" . abs( $delta ); - } else { - $sql .= "$field=$field+" . abs( $delta ); - } - } - } - - /** - * @param BagOStuff $stash - * @param string $type - * @param string $sign ('+' or '-') - * @return string - */ - private function getTypeCacheKey( BagOStuff $stash, $type, $sign ) { - return $stash->makeKey( 'sitestatsupdate', 'pendingdelta', $type, $sign ); - } - - /** - * Adjust the pending deltas for a stat type. - * Each stat type has two pending counters, one for increments and decrements - * @param string $type - * @param int $delta Delta (positive or negative) - */ - protected function adjustPending( $type, $delta ) { - if ( $delta < 0 ) { // decrement - $key = $this->getTypeCacheKey( $this->stash, $type, '-' ); - } else { // increment - $key = $this->getTypeCacheKey( $this->stash, $type, '+' ); - } - - $magnitude = abs( $delta ); - $this->stash->incrWithInit( $key, 0, $magnitude, $magnitude ); - } - - /** - * Get pending delta counters for each stat type - * @return array Positive and negative deltas for each type - */ - protected function getPendingDeltas() { - $pending = []; - foreach ( [ 'ss_total_edits', - 'ss_good_articles', 'ss_total_pages', 'ss_users', 'ss_images' ] as $type - ) { - // Get pending increments and pending decrements - $flg = BagOStuff::READ_LATEST; - $pending[$type]['+'] = (int)$this->stash->get( - $this->getTypeCacheKey( $this->stash, $type, '+' ), - $flg - ); - $pending[$type]['-'] = (int)$this->stash->get( - $this->getTypeCacheKey( $this->stash, $type, '-' ), - $flg - ); - } - - return $pending; - } - - /** - * Reduce pending delta counters after updates have been applied - * @param array $pd Result of getPendingDeltas(), used for DB update - */ - protected function removePendingDeltas( array $pd ) { - foreach ( $pd as $type => $deltas ) { - foreach ( $deltas as $sign => $magnitude ) { - // Lower the pending counter now that we applied these changes - $key = $this->getTypeCacheKey( $this->stash, $type, $sign ); - $this->stash->decr( $key, $magnitude ); - } - } - } } diff --git a/tests/phpunit/includes/deferred/SiteStatsUpdateTest.php b/tests/phpunit/includes/deferred/SiteStatsUpdateTest.php index 83e9a47ca6..ccfcc181ee 100644 --- a/tests/phpunit/includes/deferred/SiteStatsUpdateTest.php +++ b/tests/phpunit/includes/deferred/SiteStatsUpdateTest.php @@ -42,11 +42,13 @@ class SiteStatsUpdateTest extends MediaWikiTestCase { $fi = SiteStats::images(); $ai = SiteStats::articles(); + $this->assertEquals( 0, DeferredUpdates::pendingUpdatesCount() ); + $dbw->begin( __METHOD__ ); // block opportunistic updates - $update = SiteStatsUpdate::factory( [ 'pages' => 2, 'images' => 1, 'edits' => 2 ] ); - $this->assertEquals( 0, DeferredUpdates::pendingUpdatesCount() ); - $update->doUpdate(); + DeferredUpdates::addUpdate( + SiteStatsUpdate::factory( [ 'pages' => 2, 'images' => 1, 'edits' => 2 ] ) + ); $this->assertEquals( 1, DeferredUpdates::pendingUpdatesCount() ); // Still the same