Skip to content

Revert 1 main #985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
j0b333 wants to merge 2 commits into from
Closed

Revert 1 main #985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d91bd8
Merge pull request #1 from got-feedBack/main
j0b333 Jul 16, 2026
c060813
Revert "perf(scan): skip full library re-stat when the tree is unchan…
j0b333 Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 5 additions & 137 deletions lib/scan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,97 +51,6 @@

log = logging.getLogger("feedBack.scan")

import json


# ── Directory-signature fast path ─────────────────────────────────────────────
#
# A startup scan globs the whole library twice (*.feedpak, *.wem) and stats every
# file to detect what changed. On a 50k-song library that lives on a slow mount
# (an NTFS-3G FUSE volume here) it is ~100k filesystem round trips every launch —
# the "big drive churns on every startup" report.
#
# But adds / removes / renames of songs all bump the mtime of the DIRECTORY that
# holds them (verified on the target NTFS-3G mount), and so does the addition of
# a subdirectory (a new entry in its parent). So after a scan we record every
# library directory and its mtime; on the next scan we re-stat ONLY those
# directories (a handful, vs 100k file ops). If none changed, the file set is
# unchanged and the whole listing/stat pass is skipped.
#
# The one thing this cannot see is a file edited IN PLACE under the same name —
# that bumps the file's mtime but not its directory's. That is rare for a song
# library (you add and remove packs, you don't rewrite them under the same name),
# and the manual Refresh forces a full scan (force=True) for exactly that case.
def _dir_signature_file() -> Path:
return appstate.config_dir / "scan_dir_signature.json"


def _load_dir_signature() -> dict | None:
try:
data = json.loads(_dir_signature_file().read_text(encoding="utf-8"))
if isinstance(data, dict) and isinstance(data.get("dirs"), dict):
return data
except (OSError, ValueError):
pass
return None


def _save_dir_signature(dlc: Path, dirs: dict[str, int]) -> None:
# Keyed by the DLC path so switching libraries never matches a stale
# signature. Best-effort: a failed write just means the next scan is a full
# one, never a wrong one.
try:
_dir_signature_file().write_text(
json.dumps({"dlc": str(dlc), "dirs": dirs}), encoding="utf-8")
except OSError as e:
log.debug("scan: could not persist dir signature: %s", e)


def _library_dirs(all_songs, dlc: Path) -> set[str]:
"""Every directory whose mtime reflects an add/remove of a library song:
each song's containing directory and all of its ancestors up to the DLC
root (the root itself always included, as "."). Derived from the already-
listed songs — no extra filesystem walk. The builtin carve-outs
(tutorials-builtin / minigames-builtin) are absent because the caller
already excluded them from `all_songs`, so a minigame writing a drill there
never invalidates the fast path.

Directory-form songs (loose-song folders, directory sloppak bundles) also
record their OWN directory: a file added/removed/replaced INSIDE the folder
bumps that folder's mtime but not its parent's, so tracking only the parent
would miss an in-place change to such a song. File-form sloppaks (a single
.feedpak zip) aren't dirs, so they add nothing here — the flat file library
stays at a handful of dir stats."""
rels = {"."}
for f in all_songs:
rel = Path(_relpath(f, dlc))
if f.is_dir():
rels.add(rel.as_posix())
parent = rel.parent
rels.add(parent.as_posix())
for anc in parent.parents:
rels.add(anc.as_posix())
return rels


def _record_dir_signature(all_songs, dlc: Path) -> None:
sig = _stat_dirs(dlc, _library_dirs(all_songs, dlc))
if sig is not None: # a dir vanished mid-scan → skip; next scan is full
_save_dir_signature(dlc, sig)


def _stat_dirs(dlc: Path, rels) -> dict[str, int] | None:
"""{reldir: mtime_ns} for the given library dirs, or None if any is gone or
unreadable — a vanished recorded dir means the tree changed, so fail to a
full scan rather than a false match."""
out: dict[str, int] = {}
for rel in rels:
try:
out[rel] = (dlc if rel == "." else dlc / rel).stat().st_mtime_ns
except OSError:
return None
return out


_SCAN_STATUS_INIT = {"running": False, "stage": "idle", "total": 0, "done": 0, "current": "", "error": None, "is_first_scan": False, "added": 0, "removed": 0}

Expand Down Expand Up @@ -190,12 +99,9 @@ def _make_scan_executor():
)


def background_scan(force: bool = False):
def background_scan():
"""Scan the library and cache song metadata on startup. Uses a process pool to bypass the GIL for CPU-bound metadata parsing.

`force` skips the directory-signature fast path and always does the full
listing/stat pass — the manual Refresh sets it (see _dir_signature_file).

Never sets `_scan_status["running"] = False` — ownership of that flag
lives in `_scan_runner` so a `kick_scan()` racing this function's
terminal write cannot observe a stale False and start a second runner.
Expand All @@ -215,22 +121,6 @@ def background_scan(force: bool = False):
builtin_content.seed_builtin_diagnostic_sloppaks(appstate.server_root, dlc)
builtin_content.seed_builtin_starter_content(appstate.server_root, dlc)

# Fast path: if every library directory recorded by the last scan still has
# the same mtime, nothing was added, removed, or renamed, so the whole
# glob-and-stat pass below can be skipped (see the signature comment above).
# `force` (manual Refresh) always does the full pass. Seeding above is
# idempotent — it only writes when a builtin is missing — so it does not
# perturb the mtimes on a settled library.
if not force:
stored = _load_dir_signature()
if stored is not None and stored.get("dlc") == str(dlc):
current = _stat_dirs(dlc, stored["dirs"].keys())
if current is not None and current == stored["dirs"]:
_scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete"}
log.info("Scan: library tree unchanged (%d dirs) — skipped the full listing/stat pass",
len(current))
return

# Listing can fail on macOS without Full Disk Access, or on Docker if the
# path isn't shared. Report the failure explicitly rather than silently
# appearing to scan nothing.
Expand Down Expand Up @@ -333,9 +223,6 @@ def _is_excluded_from_library(p: Path) -> bool:
to_scan.append((f, mtime, size, dlc))

if not to_scan:
# Full pass completed with the DB already up to date — record the tree
# signature so the next startup can take the fast path.
_record_dir_signature(all_songs, dlc)
_scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed}
log.info("Scan: nothing new to scan (%d songs, all cached)", len(all_songs))
return
Expand All @@ -360,9 +247,6 @@ def _is_excluded_from_library(p: Path) -> bool:
_scan_status["done"] += 1
_scan_status["current"] = fname

# Record the tree signature after a completed full pass so the next startup
# can skip it when nothing has changed.
_record_dir_signature(all_songs, dlc)
log.info("Scan complete: %d songs cached", len(to_scan))
_scan_status = {**_SCAN_STATUS_INIT, "running": True, "stage": "complete", "added": added, "removed": removed}

Expand All @@ -371,9 +255,6 @@ def _is_excluded_from_library(p: Path) -> bool:


_scan_rescan_pending = False
# Set by kick_scan(force=True); consumed by _scan_runner for the next pass so a
# manual Refresh bypasses the directory-signature fast path.
_scan_force_next = False


# Handles to the running scan / enrichment worker threads. Both use the shared
Expand All @@ -384,26 +265,18 @@ def _is_excluded_from_library(p: Path) -> bool:
_scan_thread: threading.Thread | None = None


def kick_scan(force: bool = False) -> bool:
def kick_scan() -> bool:
"""Request a library rescan, single-flight + coalescing.

`force` skips the directory-signature fast path for the resulting pass (the
manual Refresh uses it so an in-place same-name edit — the one thing the
fast path can't see — is always picked up). A forced request that coalesces
onto a running or queued scan keeps the force intent: the pass is forced if
ANY pending request asked for it.

Returns True if a new scan thread was started, False if one was already
running. In the latter case a follow-up pass is queued and runs as soon
as the current scan finishes so files landing mid-scan (e.g. an upload
that finalizes after the scan has already listed DLC_DIR) are not lost
until the next periodic pass. Multiple late-arriving requests coalesce
into a single follow-up.
"""
global _scan_rescan_pending, _scan_thread, _scan_force_next
global _scan_rescan_pending, _scan_thread
with _scan_kick_lock:
if force:
_scan_force_next = True
if _scan_status["running"]:
_scan_rescan_pending = True
return False
Expand All @@ -417,15 +290,10 @@ def kick_scan(force: bool = False) -> bool:

def _scan_runner():
"""Run _background_scan, then re-run if requests arrived mid-scan."""
global _scan_rescan_pending, _scan_force_next
global _scan_rescan_pending
while True:
# Consume the force flag for THIS pass; a forced request queued mid-scan
# sets it again for the follow-up.
with _scan_kick_lock:
forced = _scan_force_next
_scan_force_next = False
try:
background_scan(force=forced)
background_scan()
except Exception:
log.exception("background scan failed unexpectedly")

Expand Down
7 changes: 2 additions & 5 deletions server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,10 +1115,7 @@ async def _gen():
@app.post("/api/rescan")
def trigger_rescan():
"""Manually trigger a library rescan."""
# force=True: a manual Refresh must skip the directory-signature fast path —
# it is the escape hatch for the one change dir mtimes can't see (a pack
# rewritten in place under the same name).
if not scan.kick_scan(force=True):
if not scan.kick_scan():
return {"message": "Scan already in progress"}
return {"message": "Rescan started"}

Expand All @@ -1136,7 +1133,7 @@ def trigger_full_rescan():
# delete_missing() prunes anything genuinely gone at the end.
meta_db.conn.execute("UPDATE songs SET mtime = -1")
meta_db.conn.commit()
if not scan.kick_scan(force=True):
if not scan.kick_scan():
return {"message": "Scan already in progress"}
return {"message": "Full rescan started"}

Expand Down
75 changes: 0 additions & 75 deletions tests/test_feedpak_extension.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,81 +137,6 @@ def mock_extract(f, dlc_dir):
assert "ignore.zip" not in seen


# ── 2b. directory-signature fast path (skip the full re-stat) ────────────────

def test_dir_signature_fast_path_skips_unchanged_tree(tmp_path, scan_server):
"""After a full scan records the library-dir signature, a second scan with
an unchanged tree takes the fast path and does NOT re-glob/extract — but a
forced scan (manual Refresh) always does the full pass, and a new song
(which bumps the dir mtime) reverts to a full pass on its own."""
import unittest.mock as mock

dlc = tmp_path / "dlc"
dlc.mkdir()
(dlc / "a.feedpak").write_bytes(b"")
(tmp_path / "config.json").write_text('{"dlc_dir": "%s"}' % dlc)

scan = importlib.import_module("scan")
seen: list[str] = []

def mock_extract(f, dlc_dir):
seen.append(f.name)
return {"title": f.name, "artist": "", "album": ""}

with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract):
# 1) first pass: full scan, extracts a.feedpak (+ seeded builtins),
# records the signature
scan.background_scan()
assert "a.feedpak" in seen
assert scan._dir_signature_file().exists()

# 2) unchanged tree: fast path — no glob, no extraction at all
seen.clear()
scan.background_scan()
assert seen == []
assert scan.status()["stage"] == "complete"

# 3) a new song bumps the dlc mtime → signature mismatch → full pass
# picks it up on its own (no manual Refresh needed for adds)
(dlc / "b.feedpak").write_bytes(b"")
seen.clear()
scan.background_scan()
assert "b.feedpak" in seen

# 4) force=True (Refresh) bypasses the fast path even on a settled tree:
# with the signature now current, a plain scan skips, a forced one lists
seen.clear()
scan.background_scan() # fast path
assert seen == []
forced_listed = []
real_delete_missing = scan.appstate.meta_db.delete_missing
def _spy(files):
forced_listed.append(set(files))
return real_delete_missing(files)
with mock.patch.object(scan.appstate.meta_db, "delete_missing", new=_spy):
scan.background_scan(force=True)
assert forced_listed, "force=True must run the full listing pass"


def test_dir_signature_tracks_directory_form_song_own_dir(tmp_path):
"""A directory-form song (loose folder / directory bundle) records its OWN
directory in the signature, so an in-place file change inside it — which
bumps that folder's mtime but not its parent's — invalidates the fast path.
A file-form sloppak (a plain .feedpak zip) is not a dir and adds nothing."""
scan = importlib.import_module("scan")
dlc = tmp_path / "dlc"
(dlc / "packs").mkdir(parents=True)
loose = dlc / "packs" / "my_loose_song" # directory-form song
loose.mkdir()
zipped = dlc / "packs" / "zipped.feedpak" # file-form song
zipped.write_bytes(b"")

rels = scan._library_dirs([loose, zipped], dlc)
assert "packs/my_loose_song" in rels, "directory-form song must track its own dir"
assert "packs" in rels and "." in rels
assert "packs/zipped.feedpak" not in rels, "a file-form sloppak is not a tracked dir"


# ── 3. POST /api/songs/upload gate (endpoint) ────────────────────────────────

@pytest.fixture()
Expand Down