From fa3e34762ad99d1a530b5f466bf91f6781fa311e Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Sat, 24 Oct 2026 23:00:23 +0200 Subject: [PATCH] perf(stats): count all active-user windows in one query getSessionStatistics() ran one COUNT per time window, eight queries that each cast configvalue over every login/lastLogin row of oc_preferences. On an instance with a million users that is eight million row reads per monitoring call. Select the eight windows as SUM(CASE WHEN ... THEN 1 ELSE 0 END) columns of a single query over the same rows. Each CASE uses the same castColumn() comparison as before, so the counts match the old per-window queries on every database, including the string and non-numeric quirks of SQLite and MySQL. SUM over no rows is NULL and maps to 0. The output keys, their order and the int values stay the same. The seen-users count in StorageStatistics already comes from IUserManager::countSeenUsers(), a plain COUNT over the (appid, configkey) index without a cast, so it stays as it is. The tests compare the result with the old per-window queries, and cover users exactly at and one second outside each cutoff, non-numeric and empty values, no lastLogin rows at all, and the seen-users count. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/SessionStatistics.php | 64 +++++++++------ tests/lib/SessionStatisticsTest.php | 123 +++++++++++++++++++++++++++- 2 files changed, 159 insertions(+), 28 deletions(-) diff --git a/lib/SessionStatistics.php b/lib/SessionStatistics.php index fa246cbf..86b0e177 100644 --- a/lib/SessionStatistics.php +++ b/lib/SessionStatistics.php @@ -31,6 +31,18 @@ class SessionStatistics { private const OFFSET_6MONTHS = 15552000; private const OFFSET_1YEAR = 31536000; + /** Seconds per reported window, keyed by the output key. */ + private const WINDOWS = [ + 'last5minutes' => self::OFFSET_5MIN, + 'last1hour' => self::OFFSET_1HOUR, + 'last24hours' => self::OFFSET_1DAY, + 'last7days' => self::OFFSET_7DAYS, + 'last1month' => self::OFFSET_1MONTH, + 'last3months' => self::OFFSET_3MONTHS, + 'last6months' => self::OFFSET_6MONTHS, + 'lastyear' => self::OFFSET_1YEAR, + ]; + private IDBConnection $connection; private ITimeFactory $timeFactory; @@ -39,40 +51,40 @@ public function __construct(IDBConnection $connection, ITimeFactory $timeFactory $this->timeFactory = $timeFactory; } - public function getSessionStatistics(): array { - return [ - 'last5minutes' => $this->getNumberOfActiveUsers(self::OFFSET_5MIN), - 'last1hour' => $this->getNumberOfActiveUsers(self::OFFSET_1HOUR), - 'last24hours' => $this->getNumberOfActiveUsers(self::OFFSET_1DAY), - 'last7days' => $this->getNumberOfActiveUsers(self::OFFSET_7DAYS), - 'last1month' => $this->getNumberOfActiveUsers(self::OFFSET_1MONTH), - 'last3months' => $this->getNumberOfActiveUsers(self::OFFSET_3MONTHS), - 'last6months' => $this->getNumberOfActiveUsers(self::OFFSET_6MONTHS), - 'lastyear' => $this->getNumberOfActiveUsers(self::OFFSET_1YEAR), - ]; - } - /** - * get number of active user in a given time span + * Count the users whose last login falls into each window, in one pass + * over the lastLogin rows. * - * @param int $offset seconds + * @return array */ - private function getNumberOfActiveUsers(int $offset): int { + public function getSessionStatistics(): array { + $now = $this->timeFactory->getTime(); $queryBuilder = $this->connection->getQueryBuilder(); - $queryBuilder->select($queryBuilder->func()->count('userid')) - ->from('preferences') - ->where($queryBuilder->expr()->eq('appid', $queryBuilder->createNamedParameter('login'))) - ->andWhere($queryBuilder->expr()->eq('configkey', $queryBuilder->createNamedParameter('lastLogin'))) - ->andwhere($queryBuilder->expr()->gte( - $queryBuilder->expr()->castColumn('configvalue', IQueryBuilder::PARAM_INT), - $queryBuilder->createNamedParameter($this->timeFactory->getTime() - $offset, IQueryBuilder::PARAM_INT), + $lastLogin = $queryBuilder->expr()->castColumn('configvalue', IQueryBuilder::PARAM_INT); + foreach (self::WINDOWS as $key => $offset) { + $isActive = $queryBuilder->expr()->gte( + $lastLogin, + $queryBuilder->createNamedParameter($now - $offset, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT, - )); + ); + $queryBuilder->selectAlias( + $queryBuilder->func()->sum($queryBuilder->createFunction('CASE WHEN ' . $isActive . ' THEN 1 ELSE 0 END')), + $key, + ); + } + $queryBuilder->from('preferences') + ->where($queryBuilder->expr()->eq('appid', $queryBuilder->createNamedParameter('login'))) + ->andWhere($queryBuilder->expr()->eq('configkey', $queryBuilder->createNamedParameter('lastLogin'))); $result = $queryBuilder->executeQuery(); - $activeUsers = (int)$result->fetchOne(); + $row = $result->fetch() ?: []; $result->closeCursor(); - return $activeUsers; + // SUM over no rows is NULL + $statistics = []; + foreach (array_keys(self::WINDOWS) as $key) { + $statistics[$key] = (int)($row[$key] ?? 0); + } + return $statistics; } } diff --git a/tests/lib/SessionStatisticsTest.php b/tests/lib/SessionStatisticsTest.php index 279b9c8d..10a776f1 100644 --- a/tests/lib/SessionStatisticsTest.php +++ b/tests/lib/SessionStatisticsTest.php @@ -10,8 +10,13 @@ namespace OCA\ServerInfo\Tests; use OCA\ServerInfo\SessionStatistics; +use OCA\ServerInfo\StorageStatistics; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\IRootFolder; +use OCP\IAppConfig; use OCP\IDBConnection; +use OCP\IUserManager; use OCP\Server; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -31,10 +36,21 @@ class SessionStatisticsTest extends TestCase { private const OFFSET_5MIN = 300; private const OFFSET_1HOUR = 3600; private const OFFSET_1DAY = 86400; + private const OFFSET_7DAYS = 604800; private const OFFSET_1MONTH = 2592000; private const OFFSET_3MONTHS = 7776000; private const OFFSET_6MONTHS = 15552000; private const OFFSET_1YEAR = 31536000; + private const WINDOWS = [ + 'last5minutes' => self::OFFSET_5MIN, + 'last1hour' => self::OFFSET_1HOUR, + 'last24hours' => self::OFFSET_1DAY, + 'last7days' => self::OFFSET_7DAYS, + 'last1month' => self::OFFSET_1MONTH, + 'last3months' => self::OFFSET_3MONTHS, + 'last6months' => self::OFFSET_6MONTHS, + 'lastyear' => self::OFFSET_1YEAR, + ]; protected function setUp(): void { parent::setUp(); @@ -82,14 +98,18 @@ private function addDummyValues(): void { } private function addDummyValuesWithLastLogin(int $offset, int $id): void { + $this->addLastLogin("session-statistics-test$id", (string)($this->currentTime - $offset + 1)); + } + + private function addLastLogin(string $userId, string $lastLogin): void { $query = $this->connection->getQueryBuilder(); $query->insert(self::TABLE) ->values( [ - 'userid' => $query->createNamedParameter("session-statistics-test$id"), + 'userid' => $query->createNamedParameter($userId), 'appid' => $query->createNamedParameter('login'), 'configkey' => $query->createNamedParameter('lastLogin'), - 'configvalue' => $query->createNamedParameter((string)($this->currentTime - $offset + 1)), + 'configvalue' => $query->createNamedParameter($lastLogin), 'lazy' => $query->createNamedParameter(0), 'type' => $query->createNamedParameter(0), 'flags' => $query->createNamedParameter(0), @@ -113,5 +133,104 @@ public function testGetSessionStatistics() { $this->assertSame(8, $result['last3months']); // 2 + 3 + 1 + 0 + 1 + 1 users in last 3 months $this->assertSame(9, $result['last6months']); // 2 + 3 + 1 + 0 + 1 + 1 + 1 users in last 6 months $this->assertSame(12, $result['lastyear']); // 2 + 3 + 1 + 0 + 1 + 1 + 1 + 3 users in last year + $this->assertSame($this->countPerWindow(), $result); + } + + public function testWindowBoundaries(): void { + $this->timeFactory->method('getTime')->willReturn($this->currentTime); + foreach (self::WINDOWS as $key => $offset) { + $this->addLastLogin("session-statistics-test-at-$key", (string)($this->currentTime - $offset)); + $this->addLastLogin("session-statistics-test-outside-$key", (string)($this->currentTime - $offset - 1)); + } + + $result = $this->instance->getSessionStatistics(); + + // the base fixtures, plus each window's user at the cutoff and the + // users just outside every smaller window + $this->assertSame([ + 'last5minutes' => 2 + 1, + 'last1hour' => 5 + 3, + 'last24hours' => 6 + 5, + 'last7days' => 6 + 7, + 'last1month' => 7 + 9, + 'last3months' => 8 + 11, + 'last6months' => 9 + 13, + 'lastyear' => 12 + 15, + ], $result); + $this->assertSame($this->countPerWindow(), $result); + } + + public function testNonNumericLastLogin(): void { + $provider = $this->connection->getDatabaseProvider(); + if ($provider === IDBConnection::PLATFORM_POSTGRES || $provider === IDBConnection::PLATFORM_ORACLE) { + $this->markTestSkipped('Casting a non-numeric lastLogin fails on this database'); + } + $this->timeFactory->method('getTime')->willReturn($this->currentTime); + $this->addLastLogin('session-statistics-test-empty', ''); + $this->addLastLogin('session-statistics-test-text', 'yesterday'); + $this->addLastLogin('session-statistics-test-mixed', ($this->currentTime - 10) . 'x'); + + $this->assertSame($this->countPerWindow(), $this->instance->getSessionStatistics()); + } + + public function testNoLastLoginRows(): void { + $this->timeFactory->method('getTime')->willReturn($this->currentTime); + $this->connection->beginTransaction(); + try { + $qb = $this->connection->getQueryBuilder(); + $qb->delete('preferences') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter('login'))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('lastLogin'))); + $qb->executeStatement(); + + $this->assertSame(array_fill_keys(array_keys(self::WINDOWS), 0), $this->instance->getSessionStatistics()); + $this->assertSame(0, $this->createStorageStatistics()->getStorageStatistics()['num_users']); + } finally { + $this->connection->rollBack(); + } + } + + public function testSeenUsersCountEveryLastLogin(): void { + $storageStatistics = $this->createStorageStatistics(); + $before = $storageStatistics->getStorageStatistics()['num_users']; + $this->addLastLogin('session-statistics-test-empty', ''); + $this->addLastLogin('session-statistics-test-old', '1'); + + $this->assertSame($before + 2, $storageStatistics->getStorageStatistics()['num_users']); + } + + private function createStorageStatistics(): StorageStatistics { + return new StorageStatistics( + $this->connection, + $this->createMock(IRootFolder::class), + $this->createMock(IAppConfig::class), + Server::get(IUserManager::class), + ); + } + + /** + * Counts each window with its own query, as a reference for the single + * pass in SessionStatistics. + * + * @return array + */ + private function countPerWindow(): array { + $counts = []; + foreach (self::WINDOWS as $key => $offset) { + $qb = $this->connection->getQueryBuilder(); + $qb->select($qb->func()->count('userid')) + ->from('preferences') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter('login'))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('lastLogin'))) + ->andWhere($qb->expr()->gte( + $qb->expr()->castColumn('configvalue', IQueryBuilder::PARAM_INT), + $qb->createNamedParameter($this->currentTime - $offset, IQueryBuilder::PARAM_INT), + IQueryBuilder::PARAM_INT, + )); + $result = $qb->executeQuery(); + $counts[$key] = (int)$result->fetchOne(); + $result->closeCursor(); + } + return $counts; } }