perf(scan): skip full library re-stat when the tree is unchanged#979
Conversation
Startup scans globbed the whole DLC tree twice (*.feedpak, *.wem) and
stat()'d every file to detect changes — ~100k filesystem round trips on
a 50k-song library, and painful on a slow NTFS-3G FUSE mount (the "big
drive churns on every launch" report).
Adds/removes/renames of songs all bump the mtime of the containing
directory (verified on the target mount), so after a full pass we persist
{reldir: mtime_ns} for every library dir (scan_dir_signature.json, keyed
by DLC path). The next scan re-stats only those dirs — a handful vs 100k
ops — and skips the entire listing/stat pass when none changed.
Blind spot: a pack rewritten in place under the same name bumps the file
mtime but not its dir's. Rare for a song library, and the manual Refresh
(/api/rescan + /api/rescan/full) now passes force=True to always do the
full pass. force threads through kick_scan -> _scan_runner and coalesces
like the rescan-pending flag.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesScan optimization and forced refresh
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RescanEndpoint
participant kick_scan
participant _scan_runner
participant background_scan
RescanEndpoint->>kick_scan: kick_scan(force=True)
kick_scan->>_scan_runner: preserve forced intent
_scan_runner->>background_scan: background_scan(force=True)
background_scan-->>_scan_runner: complete scan pass
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/scan.py`:
- Around line 100-114: The _library_dirs function currently records only each
song entry’s parent, so directory-form song roots are omitted from the fast-path
signature. When iterating over all_songs, add the relative directory entry
itself to rels whenever f.is_dir(), while preserving the existing parent and
ancestor tracking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 03d6f07c-62c3-47ca-9d44-6bd15d99b5aa
📒 Files selected for processing (3)
lib/scan.pyserver.pytests/test_feedpak_extension.py
CodeRabbit: _library_dirs recorded only each song's parent. For a directory-form song (loose-song folder or directory sloppak bundle), adding/removing/replacing a file INSIDE the folder bumps that folder's own mtime, not its parent's — so the fast path would skip a rescan it should run. Record the song's own dir when f.is_dir(). File-form sloppaks (a single .feedpak zip) aren't dirs, so the flat file library is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fixed in 20a5074 — |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
cd /repo 2>/dev/null || true
git log --oneline -5
git show 20a5074 --stat
git show 20a5074 -- lib/scan.py | head -150Length of output: 3583
✅ Action performedReview finished.
|
1 participant
Problem
Every startup scan globbed the whole DLC tree twice (
*.feedpak,*.wem) andstat()'d every file to detect changes — ~100k filesystem round trips on a 50k-song library. On a slow NTFS-3G FUSE mount this is the "big drive churns on every launch" report.Fix
Adds/removes/renames of songs all bump the mtime of the containing directory (verified on the target mount). So after a full pass we persist
{reldir: mtime_ns}for every library dir toscan_dir_signature.json(keyed by DLC path so switching libraries never matches a stale signature). The next scan re-stats only those dirs — a handful vs 100k ops — and skips the entire listing/stat pass when none changed.Blind spot + escape hatch
A pack rewritten in place under the same name bumps the file mtime but not its dir's — the fast path can't see it. Rare for a song library (you add/remove packs, you don't rewrite them under the same name). The manual Refresh covers it:
/api/rescanand/api/rescan/fullnow passforce=True, which skips the fast path and always does the full pass.forcethreads throughkick_scan→_scan_runnerand coalesces like the existing rescan-pending flag.Tests
test_dir_signature_fast_path_skips_unchanged_tree: full pass records signature → unchanged tree takes fast path (zero extraction) → new file bumps dir mtime → full pass picks it up →force=Truebypasses the fast path. Scan + career + settings suites green (169 passing).🤖 Generated with Claude Code
Summary by CodeRabbit