Store per-filter profiling in a single key

Instead of having three keys, one for total actions, one for time and
one for conditions. This has several benefits: first, it avoids race
conditions which could happen having different keys. Second, it's much
more performant. Third, the code is also clearer to understand,
and more uniform with the one for global stats.
Split from child patch by Dragons flight.

Bug: T53294
Depends-On: I1dc3be6da1cc9e03bc47e8f8c867089ad0100f6f
Change-Id: I8f47beb73cfc1b63c4b3c809fc6d65a1e66ee334
This commit is contained in:
Daimona Eaytoy 2019-02-10 11:01:39 +01:00
parent 4acb266e90
commit d04a0d3afc
2 changed files with 41 additions and 30 deletions

View file

@ -571,39 +571,35 @@ class AbuseFilter {
*/
private static function resetFilterProfile( $filter ) {
$stash = MediaWikiServices::getInstance()->getMainObjectStash();
$countKey = wfMemcKey( 'abusefilter', 'profile', $filter, 'count' );
$totalKey = wfMemcKey( 'abusefilter', 'profile', $filter, 'total' );
$condsKey = wfMemcKey( 'abusefilter', 'profile', $filter, 'conds' );
$profileKey = self::filterProfileKey( $filter );
$stash->delete( $countKey );
$stash->delete( $totalKey );
$stash->delete( $condsKey );
$stash->delete( $profileKey );
}
/**
* Retrieve per-filter statistics.
*
* @param string $filter
* @return array
*/
public static function getFilterProfile( $filter ) {
$stash = MediaWikiServices::getInstance()->getMainObjectStash();
$countKey = wfMemcKey( 'abusefilter', 'profile', $filter, 'count' );
$totalKey = wfMemcKey( 'abusefilter', 'profile', $filter, 'total' );
$condsKey = wfMemcKey( 'abusefilter', 'profile', $filter, 'conds' );
$profile = $stash->get( self::filterProfileKey( $filter ) );
$curCount = $stash->get( $countKey );
$curTotal = $stash->get( $totalKey );
$curConds = $stash->get( $condsKey );
if ( !$curCount ) {
if ( $profile !== false ) {
$curCount = $profile['count'];
$curTotalTime = $profile['total-time'];
$curTotalConds = $profile['total-cond'];
} else {
return [ 0, 0 ];
}
// 1000 ms in a sec
$timeProfile = ( $curTotal / $curCount ) * 1000;
$timeProfile = ( $curTotalTime / $curCount ) * 1000;
// Return in ms, rounded to 2dp
$timeProfile = round( $timeProfile, 2 );
$condProfile = ( $curConds / $curCount );
$condProfile = ( $curTotalConds / $curCount );
$condProfile = round( $condProfile, 0 );
return [ $timeProfile, $condProfile ];
@ -1010,6 +1006,17 @@ class AbuseFilter {
return wfMemcKey( 'abusefilter', 'stats', 'overflow' );
}
/**
* Get the memcache access key used to store per-filter profiling data.
*
* @param string|int $filter
* @return string
*/
public static function filterProfileKey( $filter ) {
$cache = MediaWikiServices::getInstance()->getMainWANObjectCache();
return $cache->makeKey( 'abusefilter-profile', 'v1', $filter );
}
/**
* @param string $group The filter's group (as defined in $wgAbuseFilterValidGroups)
* @return string

View file

@ -421,23 +421,27 @@ class AbuseFilterRunner {
// Defer updates to avoid massive (~1 second) edit time increases
DeferredUpdates::addCallableUpdate( function () use ( $filter, $time, $conds ) {
$stash = MediaWikiServices::getInstance()->getMainObjectStash();
$countKey = $stash->makeKey( 'abusefilter', 'profile', $filter, 'count' );
$totalKey = $stash->makeKey( 'abusefilter', 'profile', $filter, 'total' );
$condsKey = $stash->makeKey( 'abusefilter', 'profile', $filter, 'conds' );
$profileKey = AbuseFilter::filterProfileKey( $filter );
$profile = $stash->get( $profileKey );
$curCount = $stash->get( $countKey );
$curTotal = $stash->get( $totalKey );
$curConds = $stash->get( $condsKey );
if ( $curCount ) {
$stash->set( $condsKey, $curConds + $conds, 3600 );
$stash->set( $totalKey, $curTotal + $time, 3600 );
$stash->incr( $countKey );
if ( $profile !== false ) {
// Number of observed executions of this filter
$profile['count']++;
// Total time spent on this filter from all observed executions
$profile['total-time'] += $time;
// Total number of conditions for this filter from all executions
$profile['total-cond'] += $conds;
} else {
$stash->set( $countKey, 1, 3600 );
$stash->set( $totalKey, $time, 3600 );
$stash->set( $condsKey, $conds, 3600 );
$profile = [
'count' => 1,
'total-time' => $time,
'total-cond' => $conds
];
}
// Note: It is important that all key information be stored together in a single
// memcache entry to avoid race conditions where competing Apache instances
// partially overwrite the stats.
$stash->set( $profileKey, $profile, 3600 );
} );
}