Remove empty folders left behind by cache eviction - #202
Open
Brandon-Haney wants to merge 2 commits into
Open
Conversation
Evicting a file deleted the cache copy and stopped there, so the movie folder (and the show/season chain for TV) stayed on the cache drive. Only the normal move-to-array path cleaned up after itself. Three paths were affected: the per-file and bulk Evict buttons on the Cached Files page, the maintenance Evict action (which delegates to the same service method), and automatic smart/FIFO eviction during a run. - Adds `cleanup_empty_parent_folders()` and `resolve_cache_boundary()` to core/system_utils.py as the canonical implementations, and calls them from CacheService.evict_file() and PlexCacheApp._run_eviction(). - FileMover._cleanup_empty_parent_folders() is now a thin wrapper over the shared helper. Its boundary comes from the owning path mapping rather than the single cache_dir, so a mapping on a second pool (/mnt/ssd_cache alongside /mnt/cache) is cleaned instead of skipped, and the mapping's own cache_path is preserved as a caching destination. - The boundary comparison now requires a separator match, so /mnt/cache_downloads is no longer treated as living inside /mnt/cache. - Honours the existing cleanup_empty_folders setting on every path.
Per-file cleanup only ever sees folders it empties itself, so installs upgrading from a version where eviction skipped cleanup keep whatever backlog they already accumulated. Existing users would otherwise have to clear it by hand. - `_backfill_empty_folder_cleanup()` sweeps the enabled, cacheable cache mappings once on the first run after upgrade, then records a marker in data/migrations.json so it never repeats. Reports one summary line, `[CLEANUP] Removed N empty folder(s) left by earlier evictions`, with per-folder detail at DEBUG. - Skipped on dry runs and when cleanup_empty_folders is off. Sweep and marker failures are logged and non-fatal — housekeeping shouldn't be able to fail a run. - Adds `sweep_empty_folders()` to core/system_utils.py as the canonical full-tree sweep, and points MaintenanceService._cleanup_empty_directories() at it so there's one implementation. Dot-directories (.Trash, .Recycle.Bin) and configured excluded_folders are left alone, and the mapping roots themselves are never removed.
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.
Closes #196
Reported in #196 (comment): evicting files leaves the folders behind on the cache drive.
Turns out only the normal move-to-array path was cleaning up after itself. Three other paths deleted the cache file and stopped there:
The last one is probably the bigger source of the backlog, since it runs unattended on every run that crosses the threshold, so it has been quietly accumulating.
While wiring this up I found two other things worth mentioning:
Multi-pool setups were being skipped even on the working path. The boundary for the walk-up was the single
cache_dir, checked withstartswith. If a mapping lives on a different pool (say/mnt/ssd_cachewhilecache_diris/mnt/cache) that check fails immediately, so cleanup silently did nothing there. The boundary now comes from the owning path mapping instead.The
startswithhad no separator guard, so/mnt/cache_downloadscounted as being inside/mnt/cache. That is a cleanup walking into a directory it does not own. It now requires a separator match.One deliberate behavior change: a mapping's own
cache_pathis preserved even when it ends up empty. It is a configured caching destination rather than a folder PlexCache created, and removing it would leave the next run without somewhere to write.Since per-file cleanup only ever sees folders it empties itself, existing installs keep whatever backlog they already have. Rather than ask everyone to clear it by hand,
_backfill_empty_folder_cleanup()sweeps the cache mappings once on the first run after upgrade, records a marker indata/migrations.json, and never runs again. It logs one summary line and is skipped on dry runs and whencleanup_empty_foldersis off. Sweep and marker failures are logged but non-fatal, since housekeeping should not be able to fail a run.Also consolidated the duplicate sweep logic:
cleanup_empty_parent_folders()andsweep_empty_folders()now live incore/system_utils.py, andFileMover._cleanup_empty_parent_folders()plusMaintenanceService._cleanup_empty_directories()are thin wrappers over them.Test steps
Manual eviction:
TV, to check the walk-up:
Backfill:
[CLEANUP] Removed N empty folder(s) left by earlier evictionsshortly after startup, anddata/migrations.jsonshould gain anempty_folder_backfillkey.Opting out:
cleanup_empty_foldersin Settings, then evict a file. The folder should stay.Should be safe to leave alone: dot-directories like
.Trashand.Recycle.Bin, anything inexcluded_folders, disabled or non-cacheable mappings, and the mapping roots themselves.Adds 36 tests across
tests/test_empty_folder_cleanup.py,tests/test_evict_folder_cleanup.pyandtests/test_empty_folder_backfill.py.