peter
b7ce1d467c
- Every playlist item write (add, remove, move, reorder, rename; Plex remove and remove-bulk) row-locks the playlist before it touches an item, a move both lists in id order: remove beside reorder was a deadlock, and two adds of one video a unique-key 500. - The scheduled mirror refresh re-reads dirty under that lock, so an edit made during its YouTube calls is not replaced by YouTube's order. - A user's bulk pull holds a per-user advisory claim: two at once created duplicate mirrors. The manual sync answers 409 sync_all_busy instead of '0 synced', and like the re-pull spends its cooldown only on a pull that runs. - Deleted mirrors are committed at once, not only when a later list commits. - The claims open an unpooled connection of their own instead of a third pooled one per push.
86 lines
4.3 KiB
Python
86 lines
4.3 KiB
Python
from collections.abc import Iterator
|
|
|
|
from sqlalchemy import create_engine, text
|
|
from sqlalchemy.orm import DeclarativeBase, Session, sessionmaker
|
|
|
|
from app.config import settings
|
|
|
|
|
|
class Base(DeclarativeBase):
|
|
pass
|
|
|
|
|
|
# Pool sized above the sync-endpoint threadpool (~40) so a burst of short DB sessions can't starve on
|
|
# checkout. The image proxies deliberately DON'T hold a connection during their slow external fetch
|
|
# (see routes/plex/), so this headroom is only for the many brief sessions, not long ones. The one
|
|
# long hold — a playlist sync's advisory claim, for a whole push — opens its own unpooled connection
|
|
# (sync/playlists.py `_claim_engine`) instead of drawing a third one from here.
|
|
engine = create_engine(
|
|
settings.database_url,
|
|
pool_pre_ping=True,
|
|
pool_size=20,
|
|
max_overflow=30,
|
|
pool_timeout=30,
|
|
future=True,
|
|
# A database error renders its bound parameters into its message — a title, an uploader, a
|
|
# file path, a URL — and that message is logged and stored as a job's error text. The SQL
|
|
# stays; the values are data, and the ids an operator needs are on the line already.
|
|
hide_parameters=True,
|
|
)
|
|
SessionLocal = sessionmaker(bind=engine, autoflush=False, expire_on_commit=False)
|
|
|
|
|
|
def get_db() -> Iterator[Session]:
|
|
db = SessionLocal()
|
|
try:
|
|
yield db
|
|
finally:
|
|
db.close()
|
|
|
|
|
|
def release_read_connection(db: Session) -> None:
|
|
"""End a request's READ transaction so its pooled connection goes back BEFORE a long await.
|
|
|
|
SQLAlchemy 2.0 autobegins on the first SELECT, so a request that has merely authenticated is
|
|
already holding one of the connections above — `idle in transaction` to Postgres, and nothing
|
|
times it out. A route that then awaits something slow (a request body still arriving, an
|
|
external fetch) pins it for the whole wait, which is precisely the invariant the pool is sized
|
|
on in the comment above. Measured: the next checkout after this call reuses the very backend
|
|
that was released, and a 1.8 s upload goes from one sustained `idle in transaction` to none.
|
|
|
|
Every route that reads its own body from a dependency needs this, and `test_body_limit.py`
|
|
holds that — the three that do are the ones with a raised (or self-imposed) body ceiling, and
|
|
they got here one at a time, which is how the upload route ended up the only one fixed.
|
|
|
|
The caller must have NOTHING PENDING, and that is checked rather than described. The session is
|
|
shared with every dependency that ran before, so a rollback here would silently discard a write
|
|
one of them staged — `service.enqueue` already learned that lesson the expensive way and uses a
|
|
SAVEPOINT for it. CLAUDE.md: a rule stated in a comment must also be enforced in code.
|
|
|
|
TWO checks, because the obvious one is blind to the shapes this codebase actually writes in.
|
|
`db.new`/`deleted`/`dirty` hold only UNFLUSHED unit-of-work: the moment a helper calls
|
|
`db.flush()` — CLAUDE.md's canonical split is "the handler commits; helpers flush" — the object
|
|
is persistent with no unflushed history and all three sets are empty again, and Core DML
|
|
(`db.execute(update(...))`) never enters them at all. Measured: after `add`+`flush` the sets are
|
|
empty, the old check passed, and the rollback lost the row. So the transaction is asked
|
|
directly. Postgres assigns a transaction id on the first WRITE and never for a read, so
|
|
`pg_current_xact_id_if_assigned()` is NULL for exactly the state this function is safe in — and
|
|
it sees a raw `text("UPDATE …")` too, which no session-level bookkeeping would.
|
|
"""
|
|
if db.new or db.deleted or db.dirty:
|
|
raise RuntimeError(
|
|
"release_read_connection() with pending work: something ahead of this dependency "
|
|
"staged a write, which ending the transaction would discard. Commit it first, or do "
|
|
"it after the await."
|
|
)
|
|
if (
|
|
db.in_transaction()
|
|
and db.execute(text("SELECT pg_current_xact_id_if_assigned()")).scalar() is not None
|
|
):
|
|
raise RuntimeError(
|
|
"release_read_connection() on a transaction that has already WRITTEN: something ahead "
|
|
"of this dependency flushed a row or ran an UPDATE/INSERT/DELETE, which ending the "
|
|
"transaction would discard. Commit it first, or do it after the await."
|
|
)
|
|
db.rollback()
|