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