objectcache: tweak SqlBagOStuff purging to happen on write only

Also adjust the frequency default and put in a 100 row limit for
randomized purges. This adds a $limit parameter to the BagOStuff
method deleteObjectsExpiringBefore().

Change-Id: I66bfcc67e8e118f8c659dbbee13d54bf2cd937f5

diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php
index 7759947..1dd46f7 100644 (file)
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -655,11 +655,12 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         * @param string $date The reference date in MW format
         * @param callable|bool $progressCallback Optional, a function which will be called
         *     regularly during long-running operations with the percentage progress
-        *     as the first parameter.
+        *     as the first parameter. [optional]
+        * @param int $limit Maximum number of keys to delete [default: INF]
         *
         * @return bool Success, false if unimplemented
         */
-       public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) {
+       public function deleteObjectsExpiringBefore( $date, $progressCallback = false, $limit = INF ) {
                // stub
                return false;
        }

diff --git a/includes/libs/objectcache/CachedBagOStuff.php b/includes/libs/objectcache/CachedBagOStuff.php
index 8892f73..0bdd349 100644 (file)
--- a/includes/libs/objectcache/CachedBagOStuff.php
+++ b/includes/libs/objectcache/CachedBagOStuff.php
@@ -82,9 +82,9 @@ class CachedBagOStuff extends HashBagOStuff {
                $this->backend->setDebug( $bool );
        }
 
-       public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) {
-               parent::deleteObjectsExpiringBefore( $date, $progressCallback );
-               return $this->backend->deleteObjectsExpiringBefore( $date, $progressCallback );
+       public function deleteObjectsExpiringBefore( $date, $progressCallback = false, $limit = INF ) {
+               parent::deleteObjectsExpiringBefore( $date, $progressCallback, $limit );
+               return $this->backend->deleteObjectsExpiringBefore( $date, $progressCallback, $limit );
        }
 
        public function makeKeyInternal( $keyspace, $args ) {

diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php
index e832734..7ca04ee 100644 (file)
--- a/includes/libs/objectcache/MultiWriteBagOStuff.php
+++ b/includes/libs/objectcache/MultiWriteBagOStuff.php
@@ -208,18 +208,10 @@ class MultiWriteBagOStuff extends BagOStuff {
                return $this->caches[0]->unlock( $key );
        }
 
-       /**
-        * Delete objects expiring before a certain date.
-        *
-        * Succeed if any of the child caches succeed.
-        * @param string $date
-        * @param bool|callable $progressCallback
-        * @return bool
-        */
-       public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) {
+       public function deleteObjectsExpiringBefore( $date, $progressCallback = false, $limit = INF ) {
                $ret = false;
                foreach ( $this->caches as $cache ) {
-                       if ( $cache->deleteObjectsExpiringBefore( $date, $progressCallback ) ) {
+                       if ( $cache->deleteObjectsExpiringBefore( $date, $progressCallback, $limit ) ) {
                                $ret = true;
                        }
                }

diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php b/includes/libs/objectcache/ReplicatedBagOStuff.php
index f79c1ff..6fac0ad 100644 (file)
--- a/includes/libs/objectcache/ReplicatedBagOStuff.php
+++ b/includes/libs/objectcache/ReplicatedBagOStuff.php
@@ -108,7 +108,7 @@ class ReplicatedBagOStuff extends BagOStuff {
                return $this->writeStore->unlock( $key );
        }
 
-       public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) {
+       public function deleteObjectsExpiringBefore( $date, $progressCallback = false, $limit = INF ) {
                return $this->writeStore->deleteObjectsExpiringBefore( $date, $progressCallback );
        }
 

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index 39d0353..2ef94c4 100644 (file)
--- a/includes/objectcache/SqlBagOStuff.php
+++ b/includes/objectcache/SqlBagOStuff.php
@@ -46,7 +46,9 @@ class SqlBagOStuff extends BagOStuff {
        /** @var int */
        protected $lastExpireAll = 0;
        /** @var int */
-       protected $purgePeriod = 100;
+       protected $purgePeriod = 10;
+       /** @var int */
+       protected $purgeLimit = 100;
        /** @var int */
        protected $shards = 1;
        /** @var string */
@@ -77,12 +79,13 @@ class SqlBagOStuff extends BagOStuff {
         *                  when a cluster is replicated to another site (with different host names)
         *                  but each server has a corresponding replica in the other cluster.
         *
-        *   - purgePeriod: The average number of object cache requests in between
+        *   - purgePeriod: The average number of object cache writes in between
         *                  garbage collection operations, where expired entries
         *                  are removed from the database. Or in other words, the
         *                  reciprocal of the probability of purging on any given
-        *                  request. If this is set to zero, purging will never be
-        *                  done.
+        *                  write. If this is set to zero, purging will never be done.
+        *
+        *   - purgeLimit:  Maximum number of rows to purge at once.
         *
         *   - tableName:   The table name to use, default is "objectcache".
         *
@@ -135,6 +138,9 @@ class SqlBagOStuff extends BagOStuff {
                if ( isset( $params['purgePeriod'] ) ) {
                        $this->purgePeriod = intval( $params['purgePeriod'] );
                }
+               if ( isset( $params['purgeLimit'] ) ) {
+                       $this->purgeLimit = intval( $params['purgeLimit'] );
+               }
                if ( isset( $params['tableName'] ) ) {
                        $this->tableName = $params['tableName'];
                }
@@ -270,8 +276,6 @@ class SqlBagOStuff extends BagOStuff {
                        $keysByTable[$serverIndex][$tableName][] = $key;
                }
 
-               $this->garbageCollect(); // expire old entries if any
-
                $dataRows = [];
                foreach ( $keysByTable as $serverIndex => $serverKeys ) {
                        try {
@@ -617,7 +621,7 @@ class SqlBagOStuff extends BagOStuff {
                        // Disabled
                        return;
                }
-               // Only purge on one in every $this->purgePeriod requests.
+               // Only purge on one in every $this->purgePeriod writes
                if ( $this->purgePeriod !== 1 && mt_rand( 0, $this->purgePeriod - 1 ) ) {
                        return;
                }
@@ -625,7 +629,11 @@ class SqlBagOStuff extends BagOStuff {
                // Avoid repeating the delete within a few seconds
                if ( $now > ( $this->lastExpireAll + 1 ) ) {
                        $this->lastExpireAll = $now;
-                       $this->expireAll();
+                       $this->deleteObjectsExpiringBefore(
+                               wfTimestamp( TS_MW, $now ),
+                               false,
+                               $this->purgeLimit
+                       );
                }
        }
 
@@ -633,15 +641,15 @@ class SqlBagOStuff extends BagOStuff {
                $this->deleteObjectsExpiringBefore( wfTimestampNow() );
        }
 
-       /**
-        * Delete objects from the database which expire before a certain date.
-        * @param string $timestamp
-        * @param bool|callable $progressCallback
-        * @return bool
-        */
-       public function deleteObjectsExpiringBefore( $timestamp, $progressCallback = false ) {
+       public function deleteObjectsExpiringBefore(
+               $timestamp,
+               $progressCallback = false,
+               $limit = INF
+       ) {
                /** @noinspection PhpUnusedLocalVariableInspection */
                $silenceScope = $this->silenceTransactionProfiler();
+
+               $count = 0;
                for ( $serverIndex = 0; $serverIndex < $this->numServers; $serverIndex++ ) {
                        $db = null;
                        try {
@@ -661,7 +669,8 @@ class SqlBagOStuff extends BagOStuff {
                                                        [ 'keyname', 'exptime' ],
                                                        $conds,
                                                        __METHOD__,
-                                                       [ 'LIMIT' => 100, 'ORDER BY' => 'exptime' ] );
+                                                       [ 'LIMIT' => 100, 'ORDER BY' => 'exptime' ]
+                                               );
                                                if ( $rows === false || !$rows->numRows() ) {
                                                        break;
                                                }
@@ -684,9 +693,14 @@ class SqlBagOStuff extends BagOStuff {
                                                                'exptime < ' . $db->addQuotes( $dbTimestamp ),
                                                                'keyname' => $keys
                                                        ],
-                                                       __METHOD__ );
+                                                       __METHOD__
+                                               );
+                                               $count += $db->affectedRows();
+                                               if ( $count >= $limit ) {
+                                                       return true;
+                                               }
 
-                                               if ( $progressCallback ) {
+                                               if ( is_callable( $progressCallback ) ) {
                                                        if ( intval( $totalSeconds ) === 0 ) {
                                                                $percent = 0;
                                                        } else {
@@ -710,6 +724,7 @@ class SqlBagOStuff extends BagOStuff {
                                return false;
                        }
                }
+
                return true;
        }