[MediaWiki-commits] [Gerrit] mediawiki...ThrottleOverride[master]: Use WANCache for throttle lookups

[jenkins-bot (Code Review)]

jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/392556 )

Change subject: Use WANCache for throttle lookups
..


Use WANCache for throttle lookups

Check for throttle records in the configured WANCache instance and cache
new database lookup results on cache miss. Cached data is not purged
when the backing database record is edited, so the cache duration is low
(1 hour).

Bug: T147362
Change-Id: I11aa1eb5c13edb3638970fcba4d41a689bc106f7
---
M ThrottleOverride.hooks.php
1 file changed, 64 insertions(+), 19 deletions(-)

Approvals:
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/ThrottleOverride.hooks.php b/ThrottleOverride.hooks.php
index c52a353..6db4026 100644
--- a/ThrottleOverride.hooks.php
+++ b/ThrottleOverride.hooks.php
@@ -1,5 +1,4 @@
 
@@ -21,6 +20,9 @@
 use MediaWiki\Logger\LoggerFactory;
 
 class ThrottleOverrideHooks {
+
+   const NO_OVERRIDE = -1;
+
/**
 * @param string $ip
 * @return bool
@@ -48,32 +50,56 @@
return true;
}
 
-   $dbr = wfGetDB( DB_REPLICA );
-
if ( $user->isAnon() && IP::isValid( $user->getName() ) ) {
$ip = $user->getName();
} elseif ( $ip === null ) {
$ip = RequestContext::getMain()->getRequest()->getIP();
}
+   $hexIp = IP::toHex( $ip );
 
-   $quotedIp = $dbr->addQuotes( IP::toHex( $ip ) );
-   $conds = [
-   "thr_range_start <= $quotedIp",
-   "thr_range_end >= $quotedIp",
-   'thr_type' . $dbr->buildLike( $dbr->anyString(), 
$action, $dbr->anyString() )
-   ];
+   $cache = 
MediaWikiServices::getInstance()->getMainWANObjectCache();
+   $expiry = $cache->getWithSetCallback(
+   $cache->makeKey( 'throttle_override', $action, $hexIp ),
+   $cache::TTL_HOUR,
+   function ( $cValue, &$ttl, &$setOpts, $asOf ) use ( 
$hexIp, $action ) {
+   $dbr = wfGetDB( DB_REPLICA );
+   $setOpts += Database::getCacheSetOptions( $dbr 
);
 
-   $expiry = $dbr->selectField(
-   'throttle_override',
-   'thr_expiry',
-   $conds,
-   __METHOD__,
-   [ 'ORDER BY' => 'thr_expiry DESC' ]
+   $expiry = $dbr->selectField(
+   'throttle_override',
+   'thr_expiry',
+   ThrottleOverrideHooks::makeConds( $dbr, 
$hexIp, $action ),
+   'ThrottleOverrideHooks::onPingLimiter',
+   [ 'ORDER BY' => 'thr_expiry DESC' ]
+   );
+
+   // Its tempting to set the TTL to match the 
expiration we
+   // found in the DB, but since the record is 
editable and we do
+   // not purge every key in the range when it 
changes we will
+   // just leave the default cache time alone. The 
exception to
+   // this rule is when we are caching a row which 
will expire in
+   // less than the default TTL.
+   // NOTE: this means that changes to an existing 
record may not
+   // effect all IPs in the range equally until 
the default cache
+   // period has elapsed.
+   if ( $expiry !== false ) {
+   // An override exists; do not cache for 
more than the
+   // override's current-time-left
+   $nowUnix = time();
+   $overrideCTL = wfTimestamp( TS_UNIX, 
$expiry ) - $nowUnix;
+   $ttl = min( $ttl, max( $overrideCTL, 1 
) );
+   }
+
+   // If we return false the value will not be 
cached
+   return ( $expiry === false ) ? 
self::NO_OVERRIDE : $expiry;
+   }
);
 
-   if ( $expiry > wfTimestampNow() ) {
+   if ( $expiry === self::NO_OVERRIDE ) {
+   // We checked the database and found no record
+   return true;
+   } elseif ( $expiry > wfTimestampNow() ) {
// Valid exemption. Disable the throttle.
-
$logger = LoggerFactory::getInstance( 
'thrott

[MediaWiki-commits] [Gerrit] mediawiki...ThrottleOverride[master]: Use WANCache for throttle lookups

[BryanDavis (Code Review)]

BryanDavis has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/392556 )

Change subject: Use WANCache for throttle lookups
..

Use WANCache for throttle lookups

Check for throttle records in the configured WANCache instance and cache
new database lookup results on cache miss. Cached data is not purged
when the backing database record is edited, so the cache duration is low
(1 hour).

Bug: T147362
Change-Id: I11aa1eb5c13edb3638970fcba4d41a689bc106f7
---
D ThrottleOverride.hooks.php
A ThrottleOverrideHooks.php
M extension.json
3 files changed, 161 insertions(+), 121 deletions(-)


  git pull 
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ThrottleOverride 
refs/changes/56/392556/1

diff --git a/ThrottleOverride.hooks.php b/ThrottleOverride.hooks.php
deleted file mode 100644
index c52a353..000
--- a/ThrottleOverride.hooks.php
+++ /dev/null
@@ -1,120 +0,0 @@
-
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see .
- */
-
-use MediaWiki\Logger\LoggerFactory;
-
-class ThrottleOverrideHooks {
-   /**
-* @param string $ip
-* @return bool
-*/
-   public static function onExemptFromAccountCreationThrottle( $ip ) {
-   $result = false;
-   $user = RequestContext::getMain()->getUser();
-   return self::onPingLimiter( $user, 'actcreate', $result, $ip );
-   }
-
-   /**
-* @throws InvalidArgumentException If $action is invalid
-*
-* @param User $user
-* @param string $action
-* @param $result
-* @param null|string $ip
-*
-* @return bool
-*/
-   public static function onPingLimiter( User &$user, $action, &$result, 
$ip = null ) {
-   global $wgRateLimits;
-
-   if ( $action !== 'actcreate' && !isset( $wgRateLimits[$action] 
) ) {
-   return true;
-   }
-
-   $dbr = wfGetDB( DB_REPLICA );
-
-   if ( $user->isAnon() && IP::isValid( $user->getName() ) ) {
-   $ip = $user->getName();
-   } elseif ( $ip === null ) {
-   $ip = RequestContext::getMain()->getRequest()->getIP();
-   }
-
-   $quotedIp = $dbr->addQuotes( IP::toHex( $ip ) );
-   $conds = [
-   "thr_range_start <= $quotedIp",
-   "thr_range_end >= $quotedIp",
-   'thr_type' . $dbr->buildLike( $dbr->anyString(), 
$action, $dbr->anyString() )
-   ];
-
-   $expiry = $dbr->selectField(
-   'throttle_override',
-   'thr_expiry',
-   $conds,
-   __METHOD__,
-   [ 'ORDER BY' => 'thr_expiry DESC' ]
-   );
-
-   if ( $expiry > wfTimestampNow() ) {
-   // Valid exemption. Disable the throttle.
-
-   $logger = LoggerFactory::getInstance( 
'throttleOverride' );
-   $logger->info( 'User {user} (ip: {ip}) exempted from 
throttle {action}', [
-   'user' => $user,
-   'ip' => $ip,
-   'action' => $action,
-   ] );
-
-   $result = false;
-   return false;
-   } elseif ( $expiry !== false ) {
-   // Expired exemption. Delete it from the DB.
-   wfGetDB( DB_MASTER )->delete(
-   'throttle_override',
-   $conds,
-   __METHOD__
-   );
-   }
-
-   return true;
-   }
-
-   /**
-* @param DatabaseUpdater $updater
-* @return bool
-*/
-   public static function onLoadExtensionSchemaUpdates( DatabaseUpdater 
$updater ) {
-   $updater->addExtensionTable(
-   'throttle_override',
-   __DIR__ . '/patches/table.sql'
-   );
-   $updater->addExtensionIndex(
-   'throttle_override',
-   'thr_expiry',
-   __DIR__ . '/patches/expiry_index.sql'
-