Remove $wgSiteStatsAsyncFactor feature and related $wgMainStash use
authorAaron Schulz <aschulz@wikimedia.org>
Sat, 6 Jul 2019 20:36:43 +0000 (13:36 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Sun, 14 Jul 2019 01:53:51 +0000 (18:53 -0700)
Also include ss_row_id = 1 in the UPDATE query to avoid gap locks

Bug: T227376
Change-Id: I7b730bab05e6d8b6799b623e0aff089c1103c3c2

includes/DefaultSettings.php
includes/deferred/SiteStatsUpdate.php
tests/phpunit/includes/deferred/SiteStatsUpdateTest.php

index 0886f38..49e7a34 100644 (file)
@@ -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.
index 007cf0e..11e9337 100644 (file)
@@ -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 );
-                       }
-               }
-       }
 }
index 83e9a47..ccfcc18 100644 (file)
@@ -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