Skip to content

Commit a7b99ed

Browse files
committed
fix(mongo): honour query timeout on count() and sum() aggregate paths
count() set maxTimeMS on $options but then reassigned $options from getTransactionOptions() before the aggregate, discarding it; sum() never set it at all. A total=true document list issues count(), so its aggregation ran unbounded — a slow/large mongo read hung until the caller gave up (surfacing downstream as a ~90s 503) instead of failing fast with a Timeout. Set maxTimeMS on the options actually passed to aggregate() in both paths. Regression: testCountTimeout.
1 parent c279b24 commit a7b99ed

2 files changed

Lines changed: 56 additions & 4 deletions

File tree

src/Database/Adapter/Mongo.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2718,10 +2718,6 @@ public function count(Document $collection, array $queries = [], ?int $max = nul
27182718
$options['limit'] = $max;
27192719
}
27202720

2721-
if ($this->timeout) {
2722-
$options['maxTimeMS'] = $this->timeout;
2723-
}
2724-
27252721
// Build filters from queries
27262722
$filters = $this->buildFilters($queries);
27272723

@@ -2745,6 +2741,11 @@ public function count(Document $collection, array $queries = [], ?int $max = nul
27452741
**/
27462742

27472743
$options = $this->getTransactionOptions();
2744+
2745+
if ($this->timeout) {
2746+
$options['maxTimeMS'] = $this->timeout;
2747+
}
2748+
27482749
$pipeline = [];
27492750

27502751
// Add match stage if filters are provided
@@ -2848,6 +2849,11 @@ public function sum(Document $collection, string $attribute, array $queries = []
28482849
];
28492850

28502851
$options = $this->getTransactionOptions();
2852+
2853+
if ($this->timeout) {
2854+
$options['maxTimeMS'] = $this->timeout;
2855+
}
2856+
28512857
return $this->client->aggregate($name, $pipeline, $options)->cursor->firstBatch[0]->total ?? 0;
28522858
}
28532859

tests/e2e/Adapter/Scopes/GeneralTests.php

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,53 @@ public function testQueryTimeout(): void
8686
}
8787
}
8888

89+
public function testCountTimeout(): void
90+
{
91+
if (!$this->getDatabase()->getAdapter()->getSupportForTimeouts()) {
92+
$this->expectNotToPerformAssertions();
93+
return;
94+
}
95+
96+
/** @var Database $database */
97+
$database = $this->getDatabase();
98+
99+
$database->createCollection('count-timeouts');
100+
101+
$this->assertEquals(
102+
true,
103+
$database->createAttribute(
104+
collection: 'count-timeouts',
105+
id: 'longtext',
106+
type: Database::VAR_STRING,
107+
size: 100000000,
108+
required: true
109+
)
110+
);
89111

112+
for ($i = 0; $i < 20; $i++) {
113+
$database->createDocument('count-timeouts', new Document([
114+
'longtext' => file_get_contents(__DIR__ . '/../../../resources/longtext.txt'),
115+
'$permissions' => [
116+
Permission::read(Role::any()),
117+
Permission::update(Role::any()),
118+
Permission::delete(Role::any())
119+
]
120+
]));
121+
}
122+
123+
$database->setTimeout(1);
124+
125+
try {
126+
$database->count('count-timeouts', [
127+
Query::notEqual('longtext', 'appwrite'),
128+
]);
129+
$this->fail('count() failed to throw a timeout exception');
130+
} catch (\Exception $e) {
131+
$database->clearTimeout();
132+
$database->deleteCollection('count-timeouts');
133+
$this->assertInstanceOf(TimeoutException::class, $e);
134+
}
135+
}
90136

91137
public function testPreserveDatesUpdate(): void
92138
{

0 commit comments

Comments
 (0)