fix(userdata): read user data through to the database #32

2026-09-20T23:51:39+10:00

unkin-agent commented

2026-09-20 23:51:39 +10:00

The user data cache and the item's in-memory rows were both filled once per replica and never invalidated, so a pod serving a playback tick read its own stale resume position and wrote it back over the position another pod had just saved, silently losing resume points, played state, favourites and ratings.

drop the user item data cache
read single and batched user data from the database on every query
prefetch user data for in-memory sorts, filters and the Next Up row so each stays a fixed number of queries
default PrefetchedUserData on IUserBaseItemComparer so third-party comparers still compile
cover the lost update against real PostgreSQL with two manager instances

Closes #13

The user data cache and the item's in-memory rows were both filled once per replica and never invalidated, so a pod serving a playback tick read its own stale resume position and wrote it back over the position another pod had just saved, silently losing resume points, played state, favourites and ratings. - drop the user item data cache - read single and batched user data from the database on every query - prefetch user data for in-memory sorts, filters and the Next Up row so each stays a fixed number of queries - default PrefetchedUserData on IUserBaseItemComparer so third-party comparers still compile - cover the lost update against real PostgreSQL with two manager instances Closes #13

unkin-agent added 1 commit 2026-09-20 23:51:39 +10:00

fix(userdata): read user data through to the database

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/push/ci Pipeline was successful

Details

a7919b9bac

The user data cache and the item's in-memory rows were both filled once per
replica and never invalidated, so a pod serving a playback tick read its own
stale resume position and wrote it back over the position another pod had just
saved, silently losing resume points, played state, favourites and ratings.

- drop the user item data cache
- read single and batched user data from the database on every query
- prefetch user data for in-memory sorts and filters so each stays one query
- cover the lost update against real PostgreSQL with two manager instances

unkin-agent commented

2026-09-21 00:12:16 +10:00

Emby.Server.Implementations/TV/TVSeriesManager.cs:220,232 — DetermineNextEpisode calls _userDataManager.GetUserData(user, episode/version) per item inside a Where predicate and a foreach over versions, once per series in GetNextUpBatched; with the LRU cache gone this is now an unbatched DB round trip per candidate item per series on every Next Up request → prefetch/batch these like LibraryManager.PrefetchUserData and UserViewBuilder do (e.g. GetUserDataBatch over specials + lastWatched + nextEpisode + all versions once per GetNextUpBatched call).
MediaBrowser.Controller/Sorting/IUserBaseItemComparer.cs:38 — new required member PrefetchedUserData added to a public plugin-facing interface with no default implementation; breaks any external IUserBaseItemComparer implementation → give it a default interface implementation or document as a breaking change.
nit: Emby.Server.Implementations/Library/UserDataManager.cs:65 — SaveUserData still attaches the whole row as EntityState.Modified with no concurrency token; two ticks landing on different replicas inside the same read-modify-write window can still silently clobber each other (this PR closes the stale-cache vector, not the general write race).

- Emby.Server.Implementations/TV/TVSeriesManager.cs:220,232 — `DetermineNextEpisode` calls `_userDataManager.GetUserData(user, episode/version)` per item inside a `Where` predicate and a `foreach` over versions, once per series in `GetNextUpBatched`; with the LRU cache gone this is now an unbatched DB round trip per candidate item per series on every Next Up request → prefetch/batch these like `LibraryManager.PrefetchUserData` and `UserViewBuilder` do (e.g. `GetUserDataBatch` over specials + lastWatched + nextEpisode + all versions once per `GetNextUpBatched` call). - MediaBrowser.Controller/Sorting/IUserBaseItemComparer.cs:38 — new required member `PrefetchedUserData` added to a public plugin-facing interface with no default implementation; breaks any external `IUserBaseItemComparer` implementation → give it a default interface implementation or document as a breaking change. - nit: Emby.Server.Implementations/Library/UserDataManager.cs:65 — `SaveUserData` still attaches the whole row as `EntityState.Modified` with no concurrency token; two ticks landing on different replicas inside the same read-modify-write window can still silently clobber each other (this PR closes the stale-cache vector, not the general write race).

unkin-agent added 1 commit 2026-09-21 00:36:46 +10:00

perf(nextup): batch the user data reads the next episode selection makes

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

1c59e6afcb

Next Up walked every series in an unpaginated loop and read user data one
episode at a time, so the Home screen row cost two queries per series once the
cache was gone.

- read the played state of every candidate episode in one query
- read the versions the resume check and the last played date need in one query each
- default PrefetchedUserData on IUserBaseItemComparer so plugin comparers still compile
- cover the bounded query count and the plugin comparer with tests
- share one database across the replica tests

unkin-agent commented

2026-09-21 01:00:51 +10:00

nit: tests/Jellyfin.Server.Tests/Library/UserDataManagerReplicaTests.cs:66,98,134 — itemOnB.UserData = await LoadUserDataAsync(...) sets BaseItem.UserData, which UserDataManager.GetUserData/GetUserDataBatch no longer read (they query the DB directly); the line and its "holds the earlier row in memory" comments are vestigial and misstate what the test covers → drop the assignment or update the comment to reflect that coverage comes from the direct GetUserData calls.
nit: tests/Jellyfin.Server.Implementations.Tests/TV/TVSeriesManagerNextUpTests.cs:79 — Assert.Equal(3, batchReads) only exercises the default EnableResumable=false/EnableRewatching=false path; add cases with EnableRewatching=true and specials present to cover the query-count claim across the flag space the production code actually branches on.

- nit: tests/Jellyfin.Server.Tests/Library/UserDataManagerReplicaTests.cs:66,98,134 — `itemOnB.UserData = await LoadUserDataAsync(...)` sets `BaseItem.UserData`, which `UserDataManager.GetUserData`/`GetUserDataBatch` no longer read (they query the DB directly); the line and its "holds the earlier row in memory" comments are vestigial and misstate what the test covers → drop the assignment or update the comment to reflect that coverage comes from the direct `GetUserData` calls. - nit: tests/Jellyfin.Server.Implementations.Tests/TV/TVSeriesManagerNextUpTests.cs:79 — `Assert.Equal(3, batchReads)` only exercises the default `EnableResumable=false`/`EnableRewatching=false` path; add cases with `EnableRewatching=true` and specials present to cover the query-count claim across the flag space the production code actually branches on.

benvin merged commit 6691b785c3 into main

2026-09-21 23:13:38 +10:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/jellyfin-ha-src#32