From 481998cb423b7eecc63cd6d6c3b7055f59919341 Mon Sep 17 00:00:00 2001 From: Vishwaraj Anand Date: Fri, 3 Feb 2023 23:12:20 +0530 Subject: [PATCH] feat(Spanner): CacheSessionPool::maintain deletes sessions older than 28d --- Spanner/src/Session/CacheSessionPool.php | 24 +++- Spanner/tests/System/SessionTest.php | 3 + .../Unit/Session/CacheSessionPoolTest.php | 129 ++++++++++++++++-- 3 files changed, 142 insertions(+), 14 deletions(-) diff --git a/Spanner/src/Session/CacheSessionPool.php b/Spanner/src/Session/CacheSessionPool.php index f8b639ec3038..fba6f7148cdb 100644 --- a/Spanner/src/Session/CacheSessionPool.php +++ b/Spanner/src/Session/CacheSessionPool.php @@ -127,6 +127,7 @@ class CacheSessionPool implements SessionPoolInterface use SysvTrait; const CACHE_KEY_TEMPLATE = 'cache-session-pool.%s.%s.%s'; + const DURATION_SESSION_LIFETIME = 28*24*3600; // 28 days const DURATION_TWENTY_MINUTES = 1200; const DURATION_ONE_MINUTE = 60; const WINDOW_SIZE = 600; @@ -342,11 +343,13 @@ public function release(Session $session) $name = $session->name(); if (isset($data['inUse'][$name])) { + $creationTime = $data['inUse'][$name]['creation']; unset($data['inUse'][$name]); array_push($data['queue'], [ 'name' => $name, 'expiration' => $session->expiration() - ?: $this->time() + SessionPoolInterface::SESSION_EXPIRATION_SECONDS + ?: $this->time() + SessionPoolInterface::SESSION_EXPIRATION_SECONDS, + 'creation' => $creationTime, ]); $this->save($item->set($data)); } @@ -690,7 +693,8 @@ private function createSessions($count) foreach ($res['session'] as $result) { $sessions[] = [ 'name' => $result['name'], - 'expiration' => $this->time() + SessionPoolInterface::SESSION_EXPIRATION_SECONDS + 'expiration' => $this->time() + SessionPoolInterface::SESSION_EXPIRATION_SECONDS, + 'creation' => $this->time(), ]; $created++; @@ -711,7 +715,11 @@ private function isSessionValid(array $session) { $halfHourBeforeExpiration = $session['expiration'] - (SessionPoolInterface::SESSION_EXPIRATION_SECONDS / 2); - if ($this->time() < $halfHourBeforeExpiration) { + if (self::DURATION_SESSION_LIFETIME + $session['creation'] < + $this->time() + SessionPoolInterface::SESSION_EXPIRATION_SECONDS) { + // sessions more than 28 days old are auto deleted by server + return false; + } elseif ($this->time() < $halfHourBeforeExpiration) { return true; } elseif ($halfHourBeforeExpiration < $this->time() && $this->time() < $session['expiration']) { return $this->database @@ -904,6 +912,14 @@ public function maintain() } $sessions = $cachedData['queue']; + foreach ($sessions as $id => $session) { + if (self::DURATION_SESSION_LIFETIME + $session['creation'] < + $this->time() + SessionPoolInterface::SESSION_EXPIRATION_SECONDS) { + // sessions more than 28 days old are auto deleted by server + $this->deleteQueue += $session; + unset($sessions[$id]); + } + } // Sort sessions by expiration time, "oldest" first. // acquire() method picks sessions from the beginning of the queue, // so make sure that "oldest" ones will be picked first. @@ -960,6 +976,7 @@ public function maintain() $sessions[] = [ 'name' => $item['name'], 'expiration' => $session->expiration(), + 'creation' => $item['creation'], ]; $freshSessionsCount++; } else { @@ -991,6 +1008,7 @@ public function maintain() $sessions[] = [ 'name' => $item['name'], 'expiration' => $session->expiration(), + 'creation' => $item['creation'], ]; } } diff --git a/Spanner/tests/System/SessionTest.php b/Spanner/tests/System/SessionTest.php index 28ea2ab87011..47a23805f4ac 100644 --- a/Spanner/tests/System/SessionTest.php +++ b/Spanner/tests/System/SessionTest.php @@ -67,6 +67,9 @@ public function testCacheSessionPool() $this->assertPoolCounts($cache, $cacheKey, 0, 10, 0); + $pool->maintain(); + $this->assertPoolCounts($cache, $cacheKey, 0, 10, 0); + $exception = null; try { $pool->acquire(); diff --git a/Spanner/tests/Unit/Session/CacheSessionPoolTest.php b/Spanner/tests/Unit/Session/CacheSessionPoolTest.php index ae51c2147a66..f239ab3a0128 100644 --- a/Spanner/tests/Unit/Session/CacheSessionPoolTest.php +++ b/Spanner/tests/Unit/Session/CacheSessionPoolTest.php @@ -199,6 +199,7 @@ public function testRelease() 'session' => [ 'name' => 'session', 'expiration' => $this->time + 3600, + 'creation' => $this->time, 'lastActive' => $this->time ] ], @@ -210,7 +211,8 @@ public function testRelease() 'queue' => [ [ 'name' => 'session', - 'expiration' => $this->time + 3600 + 'expiration' => $this->time + 3600, + 'creation' => $this->time, ] ], 'inUse' => [], @@ -427,6 +429,7 @@ public function acquireDataProvider() 'session0' => [ 'name' => 'session0', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -443,7 +446,8 @@ public function acquireDataProvider() 'queue' => [ [ 'name' => 'expired', - 'expiration' => $time - 3000 + 'expiration' => $time - 3000, + 'creation' => $time ] ], 'inUse' => [], @@ -457,6 +461,7 @@ public function acquireDataProvider() 'session0' => [ 'name' => 'session0', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -476,6 +481,7 @@ public function acquireDataProvider() 'alreadyCheckedOut' => [ 'name' => 'alreadyCheckedOut', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -489,11 +495,13 @@ public function acquireDataProvider() 'session0' => [ 'name' => 'session0', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ], 'alreadyCheckedOut' => [ 'name' => 'alreadyCheckedOut', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -512,11 +520,13 @@ public function acquireDataProvider() 'expiredInUse1' => [ 'name' => 'expiredInUse1', 'expiration' => $time - 5000, + 'creation' => $time, 'lastActive' => $time - 1201 ], 'expiredInUse2' => [ 'name' => 'expiredInUse2', 'expiration' => $time - 5000, + 'creation' => $time, 'lastActive' => $time - 3601 ] ], @@ -532,6 +542,7 @@ public function acquireDataProvider() 'session0' => [ 'name' => 'session0', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -548,7 +559,8 @@ public function acquireDataProvider() 'queue' => [ [ 'name' => 'session', - 'expiration' => $time + 3600 + 'expiration' => $time + 3600, + 'creation' => $time, ] ], 'inUse' => [], @@ -562,6 +574,7 @@ public function acquireDataProvider() 'session' => [ 'name' => 'session', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -578,11 +591,13 @@ public function acquireDataProvider() 'queue' => [ [ 'name' => 'expiresSoon', - 'expiration' => $time + 1500 + 'expiration' => $time + 1500, + 'creation' => $time, ], [ 'name' => 'session', - 'expiration' => $time + 3600 + 'expiration' => $time + 3600, + 'creation' => $time, ] ], 'inUse' => [], @@ -596,6 +611,7 @@ public function acquireDataProvider() 'session' => [ 'name' => 'session', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -614,6 +630,7 @@ public function acquireDataProvider() 'inactiveInUse1' => [ 'name' => 'inactiveInUse1', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time - 1201 ] ], @@ -627,6 +644,7 @@ public function acquireDataProvider() 'inactiveInUse1' => [ 'name' => 'inactiveInUse1', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -643,19 +661,23 @@ public function acquireDataProvider() 'queue' => [ [ 'name' => 'session1', - 'expiration' => $time + 3600 + 'expiration' => $time + 3600, + 'creation' => $time, ], [ 'name' => 'session2', - 'expiration' => $time + 3600 + 'expiration' => $time + 3600, + 'creation' => $time, ], [ 'name' => 'session3', - 'expiration' => $time + 3600 + 'expiration' => $time + 3600, + 'creation' => $time, ], [ 'name' => 'session4', - 'expiration' => $time + 3600 + 'expiration' => $time + 3600, + 'creation' => $time, ] ], 'inUse' => [], @@ -669,6 +691,7 @@ public function acquireDataProvider() 'session1' => [ 'name' => 'session1', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -678,6 +701,7 @@ public function acquireDataProvider() ], $time ], + // Set #8: With labels [ [ 'labels' => [ @@ -691,6 +715,7 @@ public function acquireDataProvider() 'session0' => [ 'name' => 'session0', 'expiration' => $time + 3600, + 'creation' => $time, 'lastActive' => $time ] ], @@ -699,7 +724,58 @@ public function acquireDataProvider() 'maxInUseSessions' => 1 ], $time - ] + ], + // Set #9: Session expires in 28 days + [ + [], + [ + 'queue' => [ + [ + 'name' => 'expiredSession', + 'expiration' => $time + 3600, + 'creation' => $time + - CacheSessionPool::DURATION_SESSION_LIFETIME, + ], + [ + 'name' => 'expiresSoon', + 'expiration' => $time + 3600, + 'creation' => $time + 3600 + - CacheSessionPool::DURATION_SESSION_LIFETIME, + ], + [ + 'name' => 'activeSession', + 'expiration' => $time + 3600, + 'creation' => $time, + ] + ], + 'inUse' => [], + 'toCreate' => [], + 'windowStart' => $time, + 'maxInUseSessions' => 1 + ], + [ + 'queue' => [ + [ + 'name' => 'activeSession', + 'expiration' => $time + 3600, + 'creation' => $time, + ] + ], + 'inUse' => [ + 'expiresSoon' => [ + 'name' => 'expiresSoon', + 'expiration' => $time + 3600, + 'creation' => $time + 3600 + - CacheSessionPool::DURATION_SESSION_LIFETIME, + 'lastActive' => $time + ] + ], + 'toCreate' => [], + 'windowStart' => $time, + 'maxInUseSessions' => 1 + ], + $time + ], ]; } @@ -790,6 +866,7 @@ private function queueItem($name, $age) return [ 'name' => basename($name), 'expiration' => $this->time + 3600 - $age, + 'creation' => $this->time, ]; } @@ -884,7 +961,35 @@ public function testMaintainNoDatabase() $pool->maintain(); } - /** + /** + * @dataProvider maintainDataProvider + */ + public function testMaintainServerDeletedSessions( + $maintainInterval, + $initialItems, + $expectedItems, + $config = [], + $data = [] + ) { + $cacheData = $this->cacheData($initialItems, $maintainInterval); + $expiredTime = $this->time - 28*24*60*60; // 28 days + foreach ($cacheData['queue'] as $k => $v) { + $cacheData['queue'][$k]['creation'] = $expiredTime; + } + // all expired sessions should be deleted + $expectedItems = []; + + $cache = $this->getCacheItemPool($data + $cacheData); + $config += ['minSessions' => count($initialItems)]; + $pool = new CacheSessionPoolStub($cache, $config, $this->time); + $pool->setDatabase($this->getDatabase()); + $pool->maintain(); + $data = $pool->cacheItemPool()->getItem($this->cacheKey)->get(); + $expectedQueue = $this->queue($expectedItems); + $this->assertEquals($expectedQueue, $data['queue']); + } + + /** * @dataProvider maintainDataProvider */ public function testMaintainQueue($maintainInterval, $initialItems, $expectedItems, $config = [], $data = []) @@ -961,10 +1066,12 @@ public function testWarmupAcquireMaintain() [ 'name' => 'existing1', 'expiration' => $this->time + 3000, + 'creation' => $this->time, ], [ 'name' => 'existing2', 'expiration' => $this->time + 3000, + 'creation' => $this->time, ], ], 'inUse' => [],