From a58a0a0c6807068bfb43c8f6e9804d80f0c64ce6 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 23 Sep 2026 10:05:01 -0400 Subject: [PATCH 1/2] fix(files): chunk mounted storage IDs during scans The sharded filecache fallback can bind more than 1,000 mounted storage IDs in a single IN query. Limit each query to IQueryBuilder::MAX_IN_PARAMETERS IDs and continue through subsequent chunks until a user is found. Signed-off-by: Josh --- apps/files/lib/BackgroundJob/ScanFiles.php | 34 +++++++++++++++------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/apps/files/lib/BackgroundJob/ScanFiles.php b/apps/files/lib/BackgroundJob/ScanFiles.php index 2aa5bfa7d43b1..0c61121dded15 100644 --- a/apps/files/lib/BackgroundJob/ScanFiles.php +++ b/apps/files/lib/BackgroundJob/ScanFiles.php @@ -83,24 +83,38 @@ private function getUserToScan() { $result = $query->executeQuery(); while ($res = $result->fetchAssociative()) { if ($res['user_id']) { + $result->closeCursor(); return $res['user_id']; } } + $result->closeCursor(); // as a fallback, we try a slower approach where we find all mounted storages first // this is essentially doing the inner join manually $storages = $this->getAllMountedStorages(); + if ($storages === []) { + return false; + } - $query = $this->connection->getQueryBuilder(); - $query->select('m.user_id') - ->from('filecache', 'f') - ->leftJoin('f', 'mounts', 'm', $query->expr()->eq('m.storage_id', 'f.storage')) - ->where($query->expr()->eq('f.size', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT))) - ->andWhere($query->expr()->gt('f.parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT))) - ->andWhere($query->expr()->in('f.storage', $query->createNamedParameter($storages, IQueryBuilder::PARAM_INT_ARRAY))) - ->setMaxResults(1) - ->runAcrossAllShards(); - return $query->executeQuery()->fetchOne(); + foreach (array_chunk($storages, IQueryBuilder::MAX_IN_PARAMETERS) as $storageChunk) { + $query = $this->connection->getQueryBuilder(); + $query->select('m.user_id') + ->from('filecache', 'f') + ->leftJoin('f', 'mounts', 'm', $query->expr()->eq('m.storage_id', 'f.storage')) + ->where($query->expr()->eq('f.size', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT))) + ->andWhere($query->expr()->gt('f.parent', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT))) + ->andWhere($query->expr()->in('f.storage', $query->createNamedParameter($storageChunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->setMaxResults(1) + ->runAcrossAllShards(); + + $result = $query->executeQuery(); + $user = $result->fetchOne(); + $result->closeCursor(); + if ($user) { + return $user; + } + } + return false; } else { $query = $this->connection->getQueryBuilder(); $query->select('m.user_id') From cd0451d1687441f476a6a80860705281bc03b880 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 23 Sep 2026 10:18:55 -0400 Subject: [PATCH 2/2] test(files): cover sharded scan lookup fallback Cover early success in the initial query and mounted-storage fallback behavior for empty, multi-chunk, and unmatched storage lists. Assisted-by: Copilot:gpt-6-luna Signed-off-by: Josh --- .../tests/BackgroundJob/ScanFilesTest.php | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/apps/files/tests/BackgroundJob/ScanFilesTest.php b/apps/files/tests/BackgroundJob/ScanFilesTest.php index 389bc3c0ca75b..bda0490ece839 100644 --- a/apps/files/tests/BackgroundJob/ScanFilesTest.php +++ b/apps/files/tests/BackgroundJob/ScanFilesTest.php @@ -14,6 +14,11 @@ use OC\Files\Storage\Temporary; use OCA\Files\BackgroundJob\ScanFiles; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\DB\IResult; +use OCP\DB\QueryBuilder\IExpressionBuilder; +use OCP\DB\QueryBuilder\IParameter; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OC\DB\QueryBuilder\Sharded\ShardDefinition; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IUserMountCache; use OCP\IConfig; @@ -103,4 +108,155 @@ public function testUnscanned(): void { ->with('foouser'); $this->runJob(); } + + public function testShardedQueryFindsUserWithoutFallback(): void { + $connection = $this->createMock(IDBConnection::class); + $connection->method('getShardDefinition') + ->with('filecache') + ->willReturn($this->createMock(\OC\DB\QueryBuilder\Sharded\ShardDefinition::class)); + + $query = $this->createMock(IQueryBuilder::class); + $expressionBuilder = $this->createMock(IExpressionBuilder::class); + $result = $this->createMock(IResult::class); + + $connection->expects($this->once()) + ->method('getQueryBuilder') + ->willReturn($query); + + $query->method('expr')->willReturn($expressionBuilder); + $expressionBuilder->method('eq')->willReturn('condition'); + $expressionBuilder->method('gt')->willReturn('condition'); + + $query->method('select')->willReturnSelf(); + $query->method('from')->willReturnSelf(); + $query->method('leftJoin')->willReturnSelf(); + $query->method('where')->willReturnSelf(); + $query->method('andWhere')->willReturnSelf(); + $query->method('setMaxResults')->willReturnSelf(); + $query->method('groupBy')->willReturnSelf(); + $query->method('runAcrossAllShards')->willReturnSelf(); + $query->method('createNamedParameter') + ->willReturn($this->createMock(IParameter::class)); + $query->method('executeQuery')->willReturn($result); + + $result->expects($this->once()) + ->method('fetchAssociative') + ->willReturn(['user_id' => 'foouser']); + $result->expects($this->once()) + ->method('closeCursor') + ->willReturn(true); + + $job = new ScanFiles( + $this->createMock(IConfig::class), + $this->createMock(IEventDispatcher::class), + $this->createMock(LoggerInterface::class), + $connection, + $this->createMock(ITimeFactory::class), + $this->createMock(SetupManager::class), + $this->createMock(IUserManager::class), + ); + + $this->assertSame('foouser', self::invokePrivate($job, 'getUserToScan')); + } + + /** + * @param int[] $storages + * @param list $chunkResults One result per fallback query executed + */ + private function assertShardedFallback( + array $storages, + array $chunkResults, + string|false $expectedUser, + ): void { + $connection = $this->createMock(IDBConnection::class); + $connection->method('getShardDefinition') + ->with('filecache') + ->willReturn($this->createMock(ShardDefinition::class)); + + $firstQuery = $this->createMock(IQueryBuilder::class); + $firstResult = $this->createMock(IResult::class); + $firstResult->method('fetchAssociative')->willReturn(false); + $firstResult->expects($this->once())->method('closeCursor')->willReturn(true); + $firstQuery->method('executeQuery')->willReturn($firstResult); + + $mountQuery = $this->createMock(IQueryBuilder::class); + $mountResult = $this->createMock(IResult::class); + $mountResult->method('fetchFirstColumn')->willReturn($storages); + $mountQuery->method('executeQuery')->willReturn($mountResult); + + $boundChunks = []; + $fallbackQueries = []; + foreach ($chunkResults as $user) { + $query = $this->createMock(IQueryBuilder::class); + $result = $this->createMock(IResult::class); + $result->method('fetchOne')->willReturn($user); + $result->expects($this->once())->method('closeCursor')->willReturn(true); + $query->method('executeQuery')->willReturn($result); + $query->method('createNamedParameter') + ->willReturnCallback(function (mixed $value, mixed $type) use (&$boundChunks): string { + if (is_array($value)) { + $this->assertSame(IQueryBuilder::PARAM_INT_ARRAY, $type); + $boundChunks[] = $value; + } + return ':param'; + }); + $fallbackQueries[] = $query; + } + + $queries = [$firstQuery, $mountQuery, ...$fallbackQueries]; + $connection->expects($this->exactly(count($queries))) + ->method('getQueryBuilder') + ->willReturnOnConsecutiveCalls(...$queries); + + foreach ($queries as $query) { + $query->method('expr')->willReturn($this->createMock(IExpressionBuilder::class)); + $query->method('select')->willReturnSelf(); + $query->method('selectDistinct')->willReturnSelf(); + $query->method('from')->willReturnSelf(); + $query->method('leftJoin')->willReturnSelf(); + $query->method('where')->willReturnSelf(); + $query->method('andWhere')->willReturnSelf(); + $query->method('groupBy')->willReturnSelf(); + $query->method('setMaxResults')->willReturnSelf(); + $query->method('runAcrossAllShards')->willReturnSelf(); + } + + $job = new ScanFiles( + $this->createMock(IConfig::class), + $this->createMock(IEventDispatcher::class), + $this->createMock(LoggerInterface::class), + $connection, + $this->createMock(ITimeFactory::class), + $this->createMock(SetupManager::class), + $this->createMock(IUserManager::class), + ); + + $this->assertSame($expectedUser, self::invokePrivate($job, 'getUserToScan')); + $expectedChunks = array_slice( + array_chunk($storages, IQueryBuilder::MAX_IN_PARAMETERS), + 0, + count($chunkResults), + ); + $this->assertSame($expectedChunks, $boundChunks); + } + + public function testShardedFallbackWithNoMountedStorages(): void { + $this->assertShardedFallback([], [], false); + } + + public function testShardedFallbackFindsUserInSecondChunk(): void { + $this->assertShardedFallback( + range(1, IQueryBuilder::MAX_IN_PARAMETERS + 1), + [false, 'foouser'], + 'foouser', + ); + } + + public function testShardedFallbackChecksEveryChunkWithoutMatch(): void { + $this->assertShardedFallback( + range(1, IQueryBuilder::MAX_IN_PARAMETERS * 2 + 1), + [false, false, false], + false, + ); + } }