Aaron Schulz has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/394779 )

Change subject: Try to opportunistically flush statsd data in maintenance 
scripts
......................................................................

Try to opportunistically flush statsd data in maintenance scripts

This helps to avoid OOMs from buffer build-ups in the statsd
factory object. The piggy-backs on to the same opportunity
checks done for deferred update runs.

Bug: T181385
Change-Id: I598be98a5770f8358975815e51380c4b8f63a79e
---
M includes/GlobalFunctions.php
M includes/MediaWiki.php
M includes/libs/stats/BufferingStatsdDataFactory.php
M includes/libs/stats/IBufferingStatsdDataFactory.php
M includes/libs/stats/NullStatsdDataFactory.php
M maintenance/Maintenance.php
M tests/phpunit/MediaWikiTestCase.php
7 files changed, 66 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/79/394779/1

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index bb1951d..6a78411 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -24,7 +24,6 @@
        die( "This file is part of MediaWiki, it is not a valid entry point" );
 }
 
-use Liuggio\StatsdClient\Sender\SocketSender;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\ProcOpenError;
 use MediaWiki\Session\SessionManager;
@@ -1231,6 +1230,7 @@
 
 /**
  * @todo document
+ * @todo Move logic to MediaWiki.php
  */
 function wfLogProfilingData() {
        global $wgDebugLogGroups, $wgDebugRawPage;
@@ -1242,23 +1242,10 @@
        $profiler->setContext( $context );
        $profiler->logData();
 
-       $config = $context->getConfig();
-       $stats = MediaWikiServices::getInstance()->getStatsdDataFactory();
-       if ( $config->get( 'StatsdServer' ) && $stats->hasData() ) {
-               try {
-                       $statsdServer = explode( ':', $config->get( 
'StatsdServer' ) );
-                       $statsdHost = $statsdServer[0];
-                       $statsdPort = isset( $statsdServer[1] ) ? 
$statsdServer[1] : 8125;
-                       $statsdSender = new SocketSender( $statsdHost, 
$statsdPort );
-                       $statsdClient = new SamplingStatsdClient( 
$statsdSender, true, false );
-                       $statsdClient->setSamplingRates( $config->get( 
'StatsdSamplingRates' ) );
-                       $statsdClient->send( $stats->getData() );
-               } catch ( Exception $ex ) {
-                       MWExceptionHandler::logException( $ex );
-               }
-       }
+       // Send out any buffered statsd metrics as needed
+       ( new MediaWiki )->emitBufferedStatsdData();
 
-       # Profiling must actually be enabled...
+       // Profiling must actually be enabled...
        if ( $profiler instanceof ProfilerStub ) {
                return;
        }
diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 9e3bc10..f1baeb1 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -26,6 +26,7 @@
 use Wikimedia\Rdbms\ChronologyProtector;
 use Wikimedia\Rdbms\LBFactory;
 use Wikimedia\Rdbms\DBConnectionError;
+use Liuggio\StatsdClient\Sender\SocketSender;
 
 /**
  * The MediaWiki class is the helper class for the index.php entry point.
@@ -913,6 +914,31 @@
        }
 
        /**
+        * Send out any buffered statsd data according to sampling rules
+        *
+        * @throws ConfigException
+        * @since 1.31
+        */
+       public function emitBufferedStatsdData() {
+               $stats = 
MediaWikiServices::getInstance()->getStatsdDataFactory();
+               if ( $this->config->get( 'StatsdServer' ) && $stats->hasData() 
) {
+                       try {
+                               $statsdServer = explode( ':', 
$this->config->get( 'StatsdServer' ) );
+                               $statsdHost = $statsdServer[0];
+                               $statsdPort = isset( $statsdServer[1] ) ? 
$statsdServer[1] : 8125;
+                               $statsdSender = new SocketSender( $statsdHost, 
$statsdPort );
+                               $statsdClient = new SamplingStatsdClient( 
$statsdSender, true, false );
+                               $statsdClient->setSamplingRates( 
$this->config->get( 'StatsdSamplingRates' ) );
+                               $statsdClient->send( $stats->getData() );
+
+                               $stats->clearData(); // empty buffer for the 
next round
+                       } catch ( Exception $ex ) {
+                               MWExceptionHandler::logException( $ex );
+                       }
+               }
+       }
+
+       /**
         * Potentially open a socket and sent an HTTP request back to the server
         * to run a specified number of jobs. This registers a callback to 
cleanup
         * the socket once it's done.
diff --git a/includes/libs/stats/BufferingStatsdDataFactory.php 
b/includes/libs/stats/BufferingStatsdDataFactory.php
index d75d9c0..7108afb 100644
--- a/includes/libs/stats/BufferingStatsdDataFactory.php
+++ b/includes/libs/stats/BufferingStatsdDataFactory.php
@@ -115,6 +115,14 @@
                return $this->buffer;
        }
 
+
+       /**
+        * Clear all data from the factory
+        */
+       public function clearData() {
+               $this->buffer = [];
+       }
+
        /**
         * Set collection enable status.
         * @param bool $enabled Will collection be enabled?
diff --git a/includes/libs/stats/IBufferingStatsdDataFactory.php 
b/includes/libs/stats/IBufferingStatsdDataFactory.php
index f77b26c..c99c4f5 100644
--- a/includes/libs/stats/IBufferingStatsdDataFactory.php
+++ b/includes/libs/stats/IBufferingStatsdDataFactory.php
@@ -9,18 +9,23 @@
  */
 interface IBufferingStatsdDataFactory extends StatsdDataFactoryInterface {
        /**
-        * Check whether this data factory has any data.
+        * Check whether this data factory has any data
         * @return bool
         */
        public function hasData();
 
        /**
-        * Return data from the factory.
+        * Return data from the factory
         * @return StatsdData[]
         */
        public function getData();
 
        /**
+        * Clear all data from the factory
+        */
+       public function clearData();
+
+       /**
         * Set collection enable status.
         * @param bool $enabled Will collection be enabled?
         * @return void
diff --git a/includes/libs/stats/NullStatsdDataFactory.php 
b/includes/libs/stats/NullStatsdDataFactory.php
index ed16311..e7c3cd3 100644
--- a/includes/libs/stats/NullStatsdDataFactory.php
+++ b/includes/libs/stats/NullStatsdDataFactory.php
@@ -122,6 +122,13 @@
        }
 
        /**
+        * Clear all data from the factory
+        */
+       public function clearData() {
+
+       }
+
+       /**
         * Set collection enable status.
         * @param bool $enabled Will collection be enabled?
         * @return void
diff --git a/maintenance/Maintenance.php b/maintenance/Maintenance.php
index 255892b..47ba26b 100644
--- a/maintenance/Maintenance.php
+++ b/maintenance/Maintenance.php
@@ -591,36 +591,39 @@
                $lbFactory->setAgentName(
                        mb_strlen( $agent ) > 15 ? mb_substr( $agent, 0, 15 ) . 
'...' : $agent
                );
-               self::setLBFactoryTriggers( $lbFactory );
+               self::setLBFactoryTriggers( $lbFactory, $this->getConfig() );
        }
 
        /**
         * @param LBFactory $LBFactory
+        * @param Config $config
         * @since 1.28
         */
-       public static function setLBFactoryTriggers( LBFactory $LBFactory ) {
+       public static function setLBFactoryTriggers( LBFactory $LBFactory, 
Config $config ) {
                // Hook into period lag checks which often happen in 
long-running scripts
                $lbFactory = 
MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
                $lbFactory->setWaitForReplicationListener(
                        __METHOD__,
-                       function () {
-                               global $wgCommandLineMode;
+                       function () use ( $config ) {
                                // Check config in case of JobRunner and unit 
tests
-                               if ( $wgCommandLineMode ) {
+                               if ( $config->get( 'CommandLineMode' ) ) {
                                        
DeferredUpdates::tryOpportunisticExecute( 'run' );
                                }
+                               // Try to periodically flush buffered metrics 
to avoid OOMs
+                               ( new MediaWiki )->emitBufferedStatsdData();
                        }
                );
                // Check for other windows to run them. A script may read or do 
a few writes
                // to the master but mostly be writing to something else, like 
a file store.
                $lbFactory->getMainLB()->setTransactionListener(
                        __METHOD__,
-                       function ( $trigger ) {
-                               global $wgCommandLineMode;
+                       function ( $trigger ) use ( $config ) {
                                // Check config in case of JobRunner and unit 
tests
-                               if ( $wgCommandLineMode && $trigger === 
IDatabase::TRIGGER_COMMIT ) {
+                               if ( $config->get( 'CommandLineMode' ) && 
$trigger === IDatabase::TRIGGER_COMMIT ) {
                                        
DeferredUpdates::tryOpportunisticExecute( 'run' );
                                }
+                               // Try to periodically flush buffered metrics 
to avoid OOMs
+                               ( new MediaWiki )->emitBufferedStatsdData();
                        }
                );
        }
diff --git a/tests/phpunit/MediaWikiTestCase.php 
b/tests/phpunit/MediaWikiTestCase.php
index 4d3c37b..82b4a57 100644
--- a/tests/phpunit/MediaWikiTestCase.php
+++ b/tests/phpunit/MediaWikiTestCase.php
@@ -515,8 +515,9 @@
 
                // XXX: reset maintenance triggers
                // Hook into period lag checks which often happen in 
long-running scripts
-               $lbFactory = 
MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
-               Maintenance::setLBFactoryTriggers( $lbFactory );
+               $services = MediaWikiServices::getInstance();
+               $lbFactory = $services->getDBLoadBalancerFactory();
+               Maintenance::setLBFactoryTriggers( $lbFactory, 
$services->getMainConfig() );
 
                ob_start( 'MediaWikiTestCase::wfResetOutputBuffersBarrier' );
        }

-- 
To view, visit https://gerrit.wikimedia.org/r/394779
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I598be98a5770f8358975815e51380c4b8f63a79e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to