From 97e0939082cbdf1a3abd4f480d160d52c2fb2d79 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 22 Feb 2019 04:47:32 -0800
Subject: [PATCH] objectcache: improve set() slam prevention in
 getWithSetCallback()

* Make the SET_DELAY_HIGH_SEC check more sensitive to other types
  of delay, such as increased get() latency. Any such slowness is
  relevant to the set() stampede concern.
* Also added a statsd timing metric for this delay.

Change-Id: I053a73b40dc7e566cc59fc97aeab1a4e1bee0f28
---
 includes/libs/objectcache/WANObjectCache.php | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index 40030c3cd1..86e193bf39 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -1222,19 +1222,18 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 		$minTime = $opts['minAsOf'] ?? self::MIN_TIMESTAMP_NONE;
 		$versioned = isset( $opts['version'] );
 		$touchedCallback = $opts['touchedCallback'] ?? null;
+		$initialTime = $this->getCurrentTime();
 
 		// Get a collection name to describe this class of key
 		$kClass = $this->determineKeyClass( $key );
 
-		// Get the current key value
+		// Get the current key value and populate $curTTL and $asOf accordingly
 		$curTTL = null;
 		$cValue = $this->get( $key, $curTTL, $checkKeys, $asOf ); // current value
 		$value = $cValue; // return value
-
 		// Apply additional dynamic expiration logic if supplied
 		$curTTL = $this->applyTouchedCallback( $value, $asOf, $curTTL, $touchedCallback );
 
-		$preCallbackTime = $this->getCurrentTime();
 		// Determine if a cached value regeneration is needed or desired
 		if (
 			$this->isValid( $value, $versioned, $asOf, $minTime ) &&
@@ -1242,7 +1241,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 		) {
 			$preemptiveRefresh = (
 				$this->worthRefreshExpiring( $curTTL, $lowTTL ) ||
-				$this->worthRefreshPopular( $asOf, $ageNew, $popWindow, $preCallbackTime )
+				$this->worthRefreshPopular( $asOf, $ageNew, $popWindow, $initialTime )
 			);
 
 			if ( !$preemptiveRefresh ) {
@@ -1317,6 +1316,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 			throw new InvalidArgumentException( "Invalid cache miss callback provided." );
 		}
 
+		$preCallbackTime = $this->getCurrentTime();
 		// Generate the new value from the callback...
 		$setOpts = [];
 		++$this->callbackDepth;
@@ -1328,7 +1328,9 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 		$valueIsCacheable = ( $value !== false && $ttl >= 0 );
 
 		if ( $valueIsCacheable ) {
-			$ago = max( $this->getCurrentTime() - $preCallbackTime, 0.0 );
+			$ago = max( $this->getCurrentTime() - $initialTime, 0.0 );
+			$this->stats->timing( "wanobjectcache.$kClass.regen_set_delay", 1000 * $ago );
+
 			if ( $isKeyTombstoned ) {
 				if ( $this->checkAndSetCooloff( $key, $kClass, $ago, $lockTSE, $hasLock ) ) {
 					// When delete() is called, writes are write-holed by the tombstone,
@@ -1354,7 +1356,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
 		if ( $hasLock ) {
 			// Avoid using delete() to avoid pointless mcrouter broadcasting
-			$this->cache->changeTTL( self::MUTEX_KEY_PREFIX . $key, (int)$preCallbackTime - 60 );
+			$this->cache->changeTTL( self::MUTEX_KEY_PREFIX . $key, (int)$initialTime - 60 );
 		}
 
 		$miss = is_infinite( $minTime ) ? 'renew' : 'miss';
-- 
2.20.1