fix(cache): keep lightweight refs after a refresh instead of retaining Episode objects - #105
Draft
nickwolf wants to merge 7 commits into
Draft
fix(cache): keep lightweight refs after a refresh instead of retaining Episode objects#105nickwolf wants to merge 7 commits into
nickwolf wants to merge 7 commits into
Conversation
…t once refresh_library_cache() iterated over PlexServer.episodes(), which materializes every episode of every non-ignored show library into a single list before the loop body runs. Peak memory therefore scales with library size. On a 65,906-episode library this exceeds 1GB and the process is OOM-killed part way through the refresh, before it can complete. Because refresh_library_on_scan defaults to true, every Plex library scan retriggers it, so the container ends up in a restart loop (observed: 40 OOM kills in one day, 228 restarts). Raising the memory limit only moves the ceiling, as the allocation grows with the library. Add UnprivilegedPlexServer.iter_episodes(), which pages through each show section and yields each page before requesting the next, so pages are released as they are consumed. episodes() is kept as a thin list(iter_episodes()) wrapper for compatibility. Notes on the implementation: - maxresults is required, not redundant. Without it fetchItems() paginates internally up to the section's total size and returns it in one list, which would silently reinstate the behaviour being fixed. - The cursor advances by len(page) and stops only on an empty page. Advancing by a fixed page_size would leave a permanent gap if a short page were ever returned mid-section, and a skipped episode is not benign: it is evicted from episode_parts, so the next refresh treats it as newly added and re-applies the show's track selection over every user's manual choice. - Results are sorted by addedAt so episodes imported while the scan is running sort after the cursor, rather than shifting the offsets of unread pages. The refresh runs precisely because content was just added, so this window is real. - The unprivileged branch previously issued a library-wide query; episodes only exist in show sections, so iterating those covers the same items without asking the server for everything at once. refresh_library_cache() additionally skips collecting added/updated when the cache is cold. There is nothing to diff against, so every episode would be reported as "added" and retained to build a list that the only cold-cache caller discards. Measured against a 65,906-episode library (plexapi 4.18.1): before: OOM-killed, >1024MB, refresh never completed after: 92MB peak RSS, 65,906/65,906 episodes, no duplicates, completes in 129s The remaining growth is the episode_parts dict of part-key strings (~0.3KB per episode), not the Episode objects.
…oad per episode Episodes returned by a section search carry librarySectionTitle only on the MediaContainer, so the attribute is None on every item. Reading it made plexapi reload the full item over the network once per episode: 65,925 requests and ~19 minutes on a full-library refresh, all while holding the cache lock, and any one of them timing out aborted the refresh.
Member
|
It honestly looks all fine to me, and I don't mind keeping the test stuff as I was planning on eventually adding in tests at some point anyways so might as well get a couple tests going. You will need to fix up the merge conflicts whenever you get a chance ^^ |
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
2 participants
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.
Opening this as a draft because it stacks on #102. The first commit here is #102's, so the diff will look bigger than it really is until that one lands. No rush on reviewing it, I mainly wanted the follow-up visible rather than sitting on my fork. Once #102 merges I'll rebase and take it out of draft.
Follow-up to #102, as promised there.
#102 stopped the refresh from building one giant list of episodes, but the
addedandupdatedlists it hands back still hold fullEpisodeobjects, so the peak just moves from the fetch to whatever holds those lists. On my library (65,926 episodes) I forced the pathological case, every episode markedupdated, and the process peaked at 129.4 MiB against a 1 GB limit. Before, it was going over 1024 MiB and getting OOM-killed. The refs cost 242 bytes each.The change is a frozen
EpisodeRefdataclass carrying just the fields the post-refresh consumers actually read, plus the scheduler and status handlers updated to use it.There's a second fix in here that I think is the more interesting one. Episodes that come back from a section search have
librarySectionTitleset only on the MediaContainer, not on each Video, so the attribute isNoneon every item. plexapi treatsNoneon a partial object as "go fetch the real value" (base.py:657,if value not in (None, []): return value), which means reading that one field reloads the entire item over the network. That was 65,925 extra metadata requests and about 19 minutes per refresh, all insideself._lock, and any one of them timing out took down the whole run.iter_episodesnow stampssection.titleonto each episode before yielding, the same thing plexapi does atvideo.py:1318. Measured 200 GETs down to 0, peak 165.9 to 129.4 MiB, runtime about 20 min down to 6m22s. The existing code pays this same cost today in the scheduler'sitem.librarySectionTitle.Happy to split that stamp out into its own PR against #102 if you'd rather keep the two separate. It stands on its own and it's the smaller of the two.
Three things you should know before merging:
I've been running this on my own setup since 16 July. Nine days in, the container's memory high-water is 180 MB against a 1 GB limit, with no restarts.
Tests: there are 9 covering
EpisodeRefanditer_episodes, using fakes rather than a live server. The repo's old suite needed a real Plex server and a hardcoded episode count, and it died as collateral in 709cc76. I didn't try to revive it. If you'd rather not carry these, say so and I'll drop them.Does this look like the right approach to you?