Skip to content

Execute paged queries on the driver instead of blocking a thread - #771

Merged
KrzysztofPajak merged 2 commits into
developfrom
perf/async-query-execution
Aug 9, 2026
Merged

Execute paged queries on the driver instead of blocking a thread#771
KrzysztofPajak merged 2 commits into
developfrom
perf/async-query-execution

Conversation

@KrzysztofPajak

Copy link
Copy Markdown
Member

Type: bugfix

Issue

PagedList<T>.Create was asynchronous in name only. InitializeAsync called Count() and enumerated the IQueryable synchronously, then returned Task.CompletedTask:

TotalCount = totalCount ?? source.Count();          // blocking round trip
source = totalCount == null ? source.Skip(...).Take(pageSize) : source;
AddRange(source);                                   // second blocking round trip
return Task.CompletedTask;

Every paged listing in the application went through it — admin grids, storefront catalog listings, search — so each one held a thread pool thread for the whole duration of two database round trips. Under concurrency the thread pool becomes the constraint and has to inject threads at roughly one or two per second, which shows up as request queueing far away from the code that caused it.

The same defect existed in a second, quieter form: new PagedList<T>(query, pageIndex, pageSize) wrapped in Task.FromResult. The constructor takes IEnumerable<T>, so an IQueryable bound to it happily and executed exactly the same way, with nothing in the signature to suggest a database call was happening.

Reproduce by putting a breakpoint on any IPagedList returning service method and observing that the driver call completes on the calling thread with no await in between.

Solution

Query execution moved out of the domain project and down to the provider.

  • IRepository<T> gains ToListAsync, CountAsync and PagedAsync. All three are generic over the result type so projections are covered, not just entities.
  • MongoRepository<T> implements them on the driver's asynchronous API (MongoQueryable.ToListAsync / CountAsync).
  • LiteDBRepository<T> implements them synchronously. LiteDB is an embedded database with no asynchronous API and its Table already materialises the collection, so Task.FromResult is accurate there rather than misleading. It is commented as such.
  • PagedList<T> loses InitializeAsync and Create. It is now a plain result type built from data someone else fetched, and Grand.Domain no longer reaches the database.
  • All call sites of both forms were converted, in src/Business and in two plugins.

The repository intentionally does not check whether the query it was given is still a driver query. Falling back to enumerating an in-memory sequence would silently restore the blocking behaviour this change removes, so the driver's own ArgumentException: The source argument must be a MongoDB IQueryable is allowed through instead. MongoQueryableContractTests pins the assumption that makes this safe: filters, sorting, paging, scalar projections, groupings to anonymous types, flattened sub-collections and filters against a captured list all remain driver queries. If a driver upgrade breaks one of those, a test fails rather than a service quietly loading a whole collection into memory.

.ai/knowledge/async.md carried a rule that legitimised the pattern being removed — it stated that materialising a Table query synchronously "is fine ... because Table is IQueryable against MongoDB, not EF". That is not true and it is where the pattern kept coming from, so the rule was rewritten.

Breaking changes

Three public contract changes. None affect view models, widget zones or plugin system names.

  • IRepository<T> gains three members. There is no implementation of it outside Grand.Data in this repository, but an external plugin supplying its own implementation would no longer compile.
  • IPictureService.GetPictures returns Task<IPagedList<Picture>> instead of IPagedList<Picture>. It was synchronous while performing a blocking database query.
  • ICustomerReportService.GetBestCustomersReport returns Task<IPagedList<BestCustomerReportLine>> for the same reason.

Both service methods had two call sites each, all already inside asynchronous methods.

Testing

  1. dotnet build ./GrandNode.sln — succeeds.

  2. Start a local MongoDB on mongodb://localhost (the data test fixtures use a real instance), then run the affected test projects:

    • dotnet test src/Tests/Grand.Data.Tests/Grand.Data.Tests.csproj — 72 tests, includes the new RepositoryPagedQueryTests and MongoQueryableContractTests.
    • dotnet test src/Tests/Grand.Domain.Tests/Grand.Domain.Tests.csproj
    • The seven src/Tests/Grand.Business.*.Tests projects.
    • Grand.Web.Admin.Tests, Grand.Web.Tests, Grand.Web.Store.Tests, Grand.Module.Api.Tests, Grand.Infrastructure.Tests, Grand.Web.Common.Tests.

    Run these per project rather than across the whole solution — the full parallel run has known flakes unrelated to this change.

  3. RepositoryPagedQueryTests runs every case against both MongoDB and LiteDB: first page, last partial page, a page beyond the range, no matches, pageSize <= 0 normalisation, and a projection. It asserts TotalCount, TotalPages, HasPreviousPage and HasNextPage, which is what would break silently if the paging arithmetic drifted.

  4. Start the storefront on Kestrel (dotnet run --launch-profile Kestrel from src/Web/Grand.Web) and walk pages that go through the converted path: a category listing and its second page, /search with a query and with pagenumber=2, /newproducts, /blog. Confirm the result counts and page numbering are unchanged and nothing is logged as failed.

  5. In the admin area, page and sort a product grid, then open Settings and the picture list under Maintenance — those exercise the two service methods whose signatures changed.

🤖 Generated with Claude Code

PagedList<T>.Create was async in name only. InitializeAsync ran Count()
and enumerated the IQueryable synchronously and returned Task.CompletedTask,
so every paged listing - admin grids, catalog listings, search - held a
thread pool thread for two round trips to the database. The same defect
existed in the second form, new PagedList<T>(query, ...) wrapped in
Task.FromResult, where the constructor executed the query just as silently.

Query execution now lives with the provider rather than in the domain
project. IRepository<T> gains ToListAsync, CountAsync and PagedAsync;
MongoRepository runs them on the driver's asynchronous API and LiteDB
keeps a synchronous implementation, which is honest there because it is
an embedded database with no async API. PagedList is left as a plain
result type and no longer touches IQueryable, so Grand.Domain no longer
reaches the database.

The repository deliberately does not guard against being handed a
sequence that is already in memory. Quietly enumerating it would restore
the very pattern this removes, so the driver's own ArgumentException is
allowed through. MongoQueryableContractTests pins the assumption this
relies on - that filters, projections, groupings and flattened
sub-collections all stay driver queries.

Two service methods that were synchronous while doing blocking database
work became asynchronous as a consequence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 9, 2026 18:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

MSTEST0044: DataTestMethod is obsolete, a parametrised test is declared
with TestMethod and the data source attributes. The new paged query tests
also dropped DynamicData in favour of DataRow, which is what the rest of
the test suite uses and reads better than a method returning provider
names as strings.

The occurrence in CustomerServiceTests predates this branch and is fixed
here so the build stops reporting the warning.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@KrzysztofPajak
KrzysztofPajak merged commit 52a5df4 into develop Aug 9, 2026
5 of 6 checks passed
@KrzysztofPajak
KrzysztofPajak deleted the perf/async-query-execution branch August 9, 2026 19:07
KrzysztofPajak added a commit that referenced this pull request Aug 10, 2026
GetAllPages returned Task.FromResult(query.ToList()) from a cache acquire
lambda. Table is an IQueryable over the MongoDB driver, so ToList is a
blocking round trip dressed up as async, and blocking there is worse than
elsewhere because the cache holds a lock for the duration of the acquire.

Same treatment #771 gave the paged queries.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants