Repository navigation
Conversation
createUpdateMediaProgressFromPayload looks up the existing mediaProgress and creates one if none is found. Two requests for the same item arriving together (e.g. /api/session/local and /api/me/progress/batch/update) both miss the lookup and each create a row. Duplicates are only removed on the next server start by cleanDatabase. Queue progress updates per user and media item so each update sees the row written by the previous one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brief summary
Concurrent progress updates for the same item can each create a
mediaProgressrow. This queues progress updates per user and media item so only one row is ever created.Which issue is fixed?
Fixes #3845
May be related to #5627 and #5188.
In-depth Description
User.createUpdateMediaProgressFromPayloadloads the existing progress for the item and creates a new row if there is none. The lookup and the insert are separate awaits, andmediaProgresseshas no unique constraint on(userId, mediaItemId). Two requests for an item with no progress yet both miss the lookup, and both insert. As noted in #3845, this happens when a client calls/api/session/localand/api/me/progress/batch/updateat the same time.The duplicates stay until the next server start, when
Database.cleanDatabasedeletes the older ones. Until then, the in-memoryuser.mediaProgressesand the row a sync updates can be different rows.This change wraps the existing logic in a small per-key promise queue (
userId:episodeId|libraryItemId). Updates for the same item run one after another, so the second one finds the row created by the first and updates it. Updates for different items or users still run in parallel. A failed update doesn't block the ones queued after it, and the key is removed once its queue is empty.I went with an in-process queue rather than a unique index because the server is a single process, and an index would need a migration that first removes duplicates on existing installs. Adding the index later is still possible on top of this.
How have you tested this?
test/server/models/User.test.js:user.mediaProgresseshas one entry with the last value. On master this fails with 3 rows.npm test: 360 passing.PATCH /api/me/progress/:idfor an item with no progress created duplicate rows in 29 of 30 rounds. 4 concurrent requests created 4 rows.🤖 Generated with Claude Code