fix(subscriber): keep request status updates on the owning save's connection - #3366
Conversation
…nection This routes the subscriber DB work through event.manager so it rides the save's own connection, wrapped in event.manager.transaction() which nests as a savepoint on both the postgres and sqlite drivers, so a failure in related-request work rolls back to the savepoint and is logged as before instead of aborting the save. The detached addMovie/addSeries chains keep their own repositories since the transaction is gone by the time they run, and the per-season updates run sequentially because they now share one connection. fix #3365
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPostgreSQL connection acquisition now has a configurable timeout. Media subscribers use event-bound ChangesMedia transaction handling
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR updates database transaction handling and timeout configuration to keep related request updates on the owning save connection; no actionable merge-blocking risk remains after normal checks and review. Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant MediaSubscriber
participant EntityManager
participant MediaRequestSubscriber
participant PostgreSQL
MediaSubscriber->>EntityManager: use event-bound repository
EntityManager->>PostgreSQL: update media and related requests
MediaRequestSubscriber->>EntityManager: dispatch Radarr or Sonarr work
EntityManager->>PostgreSQL: run parent-status transaction
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes a Postgres connection-pool deadlock risk during high-concurrency media scans by ensuring subscriber-side DB work uses the same connection/transaction context as the originating save, and adds a configurable DB connection timeout to avoid silent hangs when the pool is exhausted.
Changes:
- Route
MediaSubscriberrequest/season status updates throughevent.managerand wrap them in nestedtransaction()calls (savepoints) to avoid acquiring a second pool connection. - Update
MediaRequestSubscriberhelper methods to use the event-scopedEntityManager, and isolate parent-status updates behind a savepoint. - Add
DB_CONNECT_TIMEOUT_MSconfiguration (and documentation) to bound DB connection waits.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/subscriber/MediaSubscriber.ts | Uses event.manager repositories + savepoint transactions to keep request status updates on the owning save connection (avoids pool wedge). |
| server/subscriber/MediaRequestSubscriber.ts | Passes an EntityManager into helper methods and wraps parent-status updates in a savepoint; keeps detached *arr follow-up using global repositories. |
| server/datasource.ts | Introduces DB_CONNECT_TIMEOUT_MS-driven timeout configuration for Postgres connections. |
| docs/extending-seerr/database-config.mdx | Documents the new DB_CONNECT_TIMEOUT_MS environment variable. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…e any network calls sendToRadarr now checks availability before any network call, matching sendToSonarr, so the short-circuit path does no HTTP inside the transaction at all.
|
Tested on kubernetes, with postgres 18 (CNPG). Before this fix, to not soft lock: DB_POOL_SIZE: 100With this fix, no soft lock: DB_POOL_SIZE: 10
DB_CONNECT_TIMEOUT_MS: 10000(Tested by using the image and validating that it work, with the default pool size) |
… local patch Replace the local MediaSubscriber override (ConfigMap + checksum init container) with upstream's preview-pgsql-starvation-fix image (PR seerr-team/seerr#3366, fixes #3374 and #3365). Upstream routes the subscriber work through event.manager so it rides the media save's own connection - fixing both the MEDIA_AVAILABLE drop and the pool starvation, which the local after-commit deferral could not (deferred work still acquired a second connection while the save's connection was checked out). Keeping the local patch mounted would have overwritten the fixed file in the preview image. Revert this commit to restore the local patch if the preview misbehaves; back to release tags once the PR ships. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sqlite reuses one query runner per process, so concurrent scanner saves race on its shared transaction depth counter and wedge it, silently leaving later writes uncommitted. Related-request work now runs directly on event.manager on sqlite instead of nesting a transaction.
7e79970 to
9c63bec
Compare
Description
The scanner loop could wedge the entire Postgres pool: every per-item Media save holds its connection inside an open transaction while the subscriber helpers awaited since #3223 acquire a second connection through the global
getRepository(), so with bundles of 50 against the default pool of 10 and no acquisition timeout, every connection ends up held by a save waiting on a connection that can never be freed. This routes the subscriber DB work through event.manager so it rides the save's own connection, wrapped inevent.manager.transaction()which nests as a savepoint on both the postgres and sqlite drivers, so a failure in related-request work rolls back to the savepoint and is logged as before instead of aborting the save. The detachedaddMovie/addSerieschains keep their own repositories since the transaction is gone by the time they run, and the per-season updates run sequentially because they now share one connection.I also added a db timeout configuration which defaults to 30000ms when not set in env variables.
How Has This Been Tested?
preview-pgsql-starvation-fix).Awaiting testing by the reported users.Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
Bug Fixes
Documentation