From: Aaron Schulz Date: Sat, 6 Jul 2019 07:02:13 +0000 (-0700) Subject: objectcache: tweak SqlBagOStuff purging to happen on write only X-Git-Tag: 1.34.0-rc.0~1129^2 X-Git-Url: http://git.cyclocoop.org/fichier?a=commitdiff_plain;h=c860f99a6ea9a4fe6d6bcec693f2cacf74f883b9;p=lhc%2Fweb%2Fwiklou.git 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 7759947ee0..1dd46f74a3 100644 --- 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 8892f73899..0bdd34914c 100644 --- 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 e8327344c7..7ca04ee7ca 100644 --- 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 f79c1ff65f..6fac0ade3b 100644 --- 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 39d0353d45..2ef94c4297 100644 --- 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; }