Person endpoint slow due to cartesian-explosion includes and client-side pagination #101

New Issue

Ai_Agent · 2026-07-12T19:04:33Z

Ai_Agent commented

2026-07-12 19:04:33 +00:00

Summary

GET /api/person/{id} (cosplayer detail) and GET /api/person (cosplayer search) are slow (~1s at scale of 1M assets / 25k albums / 500 cosplayers) due to three compounding issues: cartesian explosion from multi-level Include().ThenInclude() chains, client-side pagination/filtering after loading everything, and missing indexes.

Root Cause Analysis

1. Cartesian Explosion — `PersonRepository.Find()` (line 17-22)

context.People
    .Include(p => p.ProfileAsset)
    .Include(p => p.Albums)!.ThenInclude(a => a.CoverAsset)
    .Include(p => p.Albums)!.ThenInclude(a => a.Assets)!.ThenInclude(a => a.SharedWith)
    .Include(p => p.Albums)!.ThenInclude(a => a.UserOwner)
    .FirstOrDefault(p => p.Id == id);

This produces a single SQL query with two many-to-many joins (AlbumAsset + AssetUser). With ~50 albums/person × ~40 assets/album × ~3 shared-users/asset, this returns ~6,000 rows per person — SQL multiplies, not unions. All data is then paginated client-side in PersonController.Get() (lines 86-89) via .Skip().Take() on the in-memory collection.

2. Full Graph Loaded in `SearchQuery()` — `PersonRepository.SearchQuery()` (line 89-92)

The second pass of the two-pass pattern loads the full album → asset → shared-with graph for every person on the page:

var pagedPeople = context.People
    .Include(p => p.Albums)!.ThenInclude(a => a.Assets)!.ThenInclude(a => a.SharedWith)
    .Where(p => personIds.Contains(p.Id))
    .ToList();

For a page of 30 cosplayers: 30 × 50 × 40 × 3 = 180k rows transferred from DB to application — mostly wasted, since only TotalAlbums and person identity fields are used in the DTO.

3. Unnecessary Asset Loading for Admin/Curator

When accessLevel != EAccessLevel.User, visibility filtering is skipped entirely, but the full asset/shared-with graph is still loaded. Admin/curator browsing needs only person fields + album count.

4. Duplicate Visibility Filtering

The same nested visibility predicate (asset.IsPubliclyShared || asset.OwnerId == userId || asset.SharedWith.Any(u => u.Id == userId)) runs twice per person: once for filtering people (lines 96-103) and once for TotalAlbums count (lines 114-119).

5. Missing Indexes

Column	Used By	Impact
`Person.CreatedAt`	Sort-by-created in `SearchQuery()` (line 70)	Full table sort
`Album.PersonOwnerId`	`FindByPerson()`, new paginated query	Seq scan on albums
`Album.UserOwnerId`	`FindByOwner()`	Seq scan on albums

Proposed Fix

A. Single Person Endpoint — `GET /api/person/{id}`

Goal: Server-side pagination of albums with minimal data loading.

Lighten PersonRepository.Find() — remove all album includes, keep only:

context.People.Include(p => p.ProfileAsset).FirstOrDefault(p => p.Id == id);

Add paginated album-by-person method — either in AlbumRepository or as a new PersonRepository.GetAlbumsPaginated() method:
- Filters by Album.PersonOwnerId
- Applies search/sort/pagination at DB level
- Two-pass approach for user-level visibility (same pattern as AlbumRepository.SearchQuery but scoped to a person)
- Returns paginated albums + totalAlbums + totalAssets computed from the filtered set
Controller calls both methods in sequence (2 round-trips, each lightweight) instead of one massive load.

B. Search People — `GET /api/person`

Goal: Don't load assets/shared-with unless necessary.

Branch second pass by access level:
- Admin/Curator: Load only albums (no assets). TotalAlbums = p.Albums.Count.
```
context.People.Include(p => p.Albums).Where(p => personIds.Contains(p.Id)).ToList();
```
- User: Keep two-pass with includes, but see optimization C below.

Project in first pass instead of second for admin/curator:

Could even skip the second pass entirely and use Select() projection in the first pass:

var dtos = ordered
    .Skip(pageOffset).Take(pageSize)
    .Select(p => new PersonPreviewDto { Id = p.Id, Name = p.Name, ... })
    .ToList();

C. User-Level Visibility Optimization

Instead of loading Assets → SharedWith inline (cartesian explosion), use batch resolution:

Load albums + assets for the page of people
Batch query AssetUser for those asset IDs + current userId
Resolve visibility in C# from the two result sets

This reduces the join from 3 levels → 2 levels and avoids the cartesian product with AssetUser.

D. Add Missing Indexes

Add fluent API indexes in LactoseDbContext.OnModelCreating:

modelBuilder.Entity<Person>().HasIndex(p => p.CreatedAt)
    .HasDatabaseName("IX_People_CreatedAt");

modelBuilder.Entity<Album>().HasIndex(a => a.PersonOwnerId)
    .HasDatabaseName("IX_Albums_PersonOwnerId");

modelBuilder.Entity<Album>().HasIndex(a => a.UserOwnerId)
    .HasDatabaseName("IX_Albums_UserOwnerId");

E. Eliminate Duplicate Filtering (Bonus)

Memoize or restructure SearchQuery's visibility check so it runs once per person and the result is re-used for both person-filtering and TotalAlbums counting.

Implementation Order

Add indexes (Part D) — safe, zero code change, immediate benefit
Refactor GET /api/person/{id} (Part A) — split find + paginated album query
Optimize SearchQuery for admin/curator (Part B)
Optimize user-level visibility (Part C)
Eliminate duplicate filtering (Part E)

Affected Files

Lactose/Repositories/PersonRepository.cs
Lactose/Repositories/IPersonRepository.cs
Lactose/Repositories/AlbumRepository.cs (may need new method)
Lactose/Repositories/IAlbumRepository.cs (interface)
Lactose/Controllers/PersonController.cs
Lactose/Mapper/PersonMapper.cs
Lactose/Context/LactoseDbContext.cs (indexes)
Butter/Dtos/Person/PersonDetailedDto.cs (may need to add pagination metadata)

## Summary `GET /api/person/{id}` (cosplayer detail) and `GET /api/person` (cosplayer search) are slow (~1s at scale of 1M assets / 25k albums / 500 cosplayers) due to three compounding issues: **cartesian explosion** from multi-level `Include().ThenInclude()` chains, **client-side pagination/filtering** after loading everything, and **missing indexes**. ## Root Cause Analysis ### 1. Cartesian Explosion — `PersonRepository.Find()` (line 17-22) ```csharp context.People .Include(p => p.ProfileAsset) .Include(p => p.Albums)!.ThenInclude(a => a.CoverAsset) .Include(p => p.Albums)!.ThenInclude(a => a.Assets)!.ThenInclude(a => a.SharedWith) .Include(p => p.Albums)!.ThenInclude(a => a.UserOwner) .FirstOrDefault(p => p.Id == id); ``` This produces a single SQL query with **two many-to-many joins** (`AlbumAsset` + `AssetUser`). With ~50 albums/person × ~40 assets/album × ~3 shared-users/asset, this returns **~6,000 rows per person** — SQL multiplies, not unions. All data is then paginated **client-side** in `PersonController.Get()` (lines 86-89) via `.Skip().Take()` on the in-memory collection. ### 2. Full Graph Loaded in `SearchQuery()` — `PersonRepository.SearchQuery()` (line 89-92) The second pass of the two-pass pattern loads the full album → asset → shared-with graph for every person on the page: ```csharp var pagedPeople = context.People .Include(p => p.Albums)!.ThenInclude(a => a.Assets)!.ThenInclude(a => a.SharedWith) .Where(p => personIds.Contains(p.Id)) .ToList(); ``` For a page of 30 cosplayers: **30 × 50 × 40 × 3 = 180k rows** transferred from DB to application — mostly wasted, since only `TotalAlbums` and person identity fields are used in the DTO. ### 3. Unnecessary Asset Loading for Admin/Curator When `accessLevel != EAccessLevel.User`, visibility filtering is skipped entirely, but the full asset/shared-with graph is still loaded. Admin/curator browsing needs only person fields + album count. ### 4. Duplicate Visibility Filtering The same nested visibility predicate (`asset.IsPubliclyShared || asset.OwnerId == userId || asset.SharedWith.Any(u => u.Id == userId)`) runs **twice** per person: once for filtering people (lines 96-103) and once for `TotalAlbums` count (lines 114-119). ### 5. Missing Indexes | Column | Used By | Impact | |--------|---------|--------| | `Person.CreatedAt` | Sort-by-created in `SearchQuery()` (line 70) | Full table sort | | `Album.PersonOwnerId` | `FindByPerson()`, new paginated query | Seq scan on albums | | `Album.UserOwnerId` | `FindByOwner()` | Seq scan on albums | ## Proposed Fix ### A. Single Person Endpoint — `GET /api/person/{id}` **Goal:** Server-side pagination of albums with minimal data loading. 1. **Lighten `PersonRepository.Find()`** — remove all album includes, keep only: ```csharp context.People.Include(p => p.ProfileAsset).FirstOrDefault(p => p.Id == id); ``` 2. **Add paginated album-by-person method** — either in `AlbumRepository` or as a new `PersonRepository.GetAlbumsPaginated()` method: - Filters by `Album.PersonOwnerId` - Applies search/sort/pagination at DB level - Two-pass approach for user-level visibility (same pattern as `AlbumRepository.SearchQuery` but scoped to a person) - Returns paginated albums + `totalAlbums` + `totalAssets` computed from the filtered set 3. **Controller** calls both methods in sequence (2 round-trips, each lightweight) instead of one massive load. ### B. Search People — `GET /api/person` **Goal:** Don't load assets/shared-with unless necessary. 1. **Branch second pass by access level:** - **Admin/Curator:** Load only albums (no assets). `TotalAlbums` = `p.Albums.Count`. ```csharp context.People.Include(p => p.Albums).Where(p => personIds.Contains(p.Id)).ToList(); ``` - **User:** Keep two-pass with includes, but see optimization C below. 2. **Project in first pass instead of second for admin/curator:** - Could even skip the second pass entirely and use `Select()` projection in the first pass: ```csharp var dtos = ordered .Skip(pageOffset).Take(pageSize) .Select(p => new PersonPreviewDto { Id = p.Id, Name = p.Name, ... }) .ToList(); ``` ### C. User-Level Visibility Optimization Instead of loading `Assets → SharedWith` inline (cartesian explosion), use **batch resolution**: 1. Load albums + assets for the page of people 2. Batch query `AssetUser` for those asset IDs + current userId 3. Resolve visibility in C# from the two result sets This reduces the join from 3 levels → 2 levels and avoids the cartesian product with `AssetUser`. ### D. Add Missing Indexes Add fluent API indexes in `LactoseDbContext.OnModelCreating`: ```csharp modelBuilder.Entity<Person>().HasIndex(p => p.CreatedAt) .HasDatabaseName("IX_People_CreatedAt"); modelBuilder.Entity<Album>().HasIndex(a => a.PersonOwnerId) .HasDatabaseName("IX_Albums_PersonOwnerId"); modelBuilder.Entity<Album>().HasIndex(a => a.UserOwnerId) .HasDatabaseName("IX_Albums_UserOwnerId"); ``` ### E. Eliminate Duplicate Filtering (Bonus) Memoize or restructure `SearchQuery`'s visibility check so it runs once per person and the result is re-used for both person-filtering and `TotalAlbums` counting. ## Implementation Order 1. **Add indexes** (Part D) — safe, zero code change, immediate benefit 2. **Refactor `GET /api/person/{id}`** (Part A) — split find + paginated album query 3. **Optimize `SearchQuery`** for admin/curator (Part B) 4. **Optimize user-level visibility** (Part C) 5. **Eliminate duplicate filtering** (Part E) ## Affected Files - `Lactose/Repositories/PersonRepository.cs` - `Lactose/Repositories/IPersonRepository.cs` - `Lactose/Repositories/AlbumRepository.cs` (may need new method) - `Lactose/Repositories/IAlbumRepository.cs` (interface) - `Lactose/Controllers/PersonController.cs` - `Lactose/Mapper/PersonMapper.cs` - `Lactose/Context/LactoseDbContext.cs` (indexes) - `Butter/Dtos/Person/PersonDetailedDto.cs` (may need to add pagination metadata)

Ai_Agent added the performance page:cosplayers page:cosplayer-detail area:backend area:db priority:high labels 2026-07-12 19:04:33 +00:00

REDCODE referenced this issue

2026-07-13 13:14:08 +00:00

Global "Search Anything" typeahead dropdown in navbar #91

REDCODE added this to the v1.1 - Upload, Admin Tooling & Site Quality milestone 2026-07-13 17:16:57 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MilkyShots/MilkyShots#101

Person endpoint slow due to cartesian-explosion includes and client-side pagination #101