fix(cache): stream library refresh instead of loading every episode at once - #102
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.
Yes it looks acceptable to me, and will gladly be appreciated and merged in hopefully soon.
No need to do so unless you think you can do/make a better approach at it before I merge in this.
Yea sorry about that I've been meaning to set up tests again at some point in the repo so I could stop needing to test everything against my own machine, I just really haven't had time honestly to do so yet. Feel free to post the if you want, but I plan on just running things when I get a chance against my own server since it has grown a lot over the past month, though I'll probably still end up merging this in regardless if I can reproduce things or not and let it sit in main a bit before pushing an actual release. Anyways as noted in other threads I am/have pretty busy and will be for the next couple months so I don't have a whole lot of time to sit down and really check issues/prs (especially testing them to make sure things are good and such) right now, so at the moment everything with this repo is kind of just on hold sadly until I have some free time to actually sit down and look over things, test things and such for any of the current open issues/prs. |
|
Sounds good! I'm running this on my end locally and I'll report back if anything changes beyond what I've found. |
|
Looks like I might have a tiny bit of free time next week to sit down and test this PR out a bit and possibly merge it in. You haven't run into any issues due to this PR so far while waiting though hopefully? |
|
Yup, still running smoothly on my end. In the last 8 days, the RAM high water mark is only 180MG, instead of the 1G it was hitting prior. Still seems to be working well. |
2 participants
refresh_library_cache()loads the entire library into memory before it starts iterating, so peak memory scales with library size. On my server (65,906 episodes after ignores) that's enough to get the process OOM-killed before the refresh finishes, and sincerefresh_library_on_scandefaults to true, every Plex scan starts it again. I ended up at 40 OOM kills in one day and a RestartCount of 228.I spent a while assuming this was just my memory limit being too low. Bumped 512M to 1G, and it died at 1G too:
It took me embarrassingly long to even confirm these were OOMs. Docker reported
OOMKilled: false, the healthcheck stayed green the whole time, anddocker statsshowed 19% because I kept sampling minutes after a restart, when it sits flat at ~116M. The kills are only visible in the host's dmesg (I'm on a nested LXC, so the container's own cgroup counters read clean). I also burned time on theAttributeError: 'NoneType' object has no attribute 'replace'in plexapi'sEpisode.__repr__that shows up in my logs. That one's real but separate, and it isn't what's killing the process.The cause is
episodes().section.all(libtype="episode", container_size=1000)already pages the HTTP requests in 1000s, butepisodes.extend(...)accumulates every page into one list, and nothing is released until the refresh returns. That's fine on a library that fits in RAM. Mine doesn't, and there's no limit I can set that fixes it, because the list grows with the library and I just hit the new ceiling instead.refresh_library_cache()only needs one episode at a time. So this addsiter_episodes(), which pages each show section and yields each page before requesting the next, and leavesepisodes()as alist(iter_episodes())wrapper. Measured on the same 65,906-episode library, plexapi 4.18.1:The 92MB is mostly the
episode_partsdict of part-key strings, roughly 0.3KB per episode. I haven't taken a heap snapshot, so I can't claim that's 100% of what remains.Four things in the diff that aren't obvious:
maxresults=page_sizemight look redundant next tocontainer_size, but without itfetchItems()keeps paginating internally up to the section's total size and hands back the whole thing in one list, which is exactly what this is trying to avoid.The cursor advances by
len(page)and stops only on an empty page, rather than advancing by a fixedpage_size. A short page mid-section would otherwise leave a permanent gap, and a skipped episode isn't harmless: it drops out ofepisode_parts, so the next refresh sees it as newly added and reapplies the show's track selection over whatever the user picked manually.Results are sorted by
addedAtso episodes imported while the scan is running land after the cursor instead of shifting the offsets of pages that haven't been read yet. The refresh runs because something was just added, so that window isn't theoretical. Offset paging can still shift on deletes; I don't think there's a fix for that short of keyset pagination, which Plex doesn't offer.The unprivileged branch used to do a library-wide
library.all(libtype="episode"). Episodes only live in show sections, so iterating those covers the same items. It also avoids a whole-library query that reliably times out at 30s on my server.One related change:
refresh_library_cache()now skips collectingadded/updatedwhen the cache is cold. There's nothing to diff against at that point, so every episode counts as "added" and gets retained to build a list that the cold-cache caller in__init__discards anyway.Something I did not fix here: in the normal scan path,
added/updatedstill hold full Episode objects. If a bulk operation changes part keys across the library (quality upgrades, a path remap, a reimport), everything lands inupdatedand you're back around 790MB on a library this size. Fixing that means collecting lightweight records and having the callers refetch, which changes the contract instatus.pyandplex_server.py. That's a bigger change and it'd bury this one, so I'll send it as a second PR once this is settled.Does this approach look acceptable to you? I'm happy to rework it if you'd rather solve it a different way. There are no tests in the repo so I verified against a live server; I can share the paging-correctness check I ran (pages disjoint, no overlap, stable order across calls) if that's useful.
This may also be what's behind #39.