Skip to content

refresh stale stats across inode rotation (Phase 3) - #736

Merged
ajslater merged 1 commit into
developfrom
fix/refresh-stale-stats-on-inode-rotation
May 7, 2026
Merged

ajslater merged 1 commit into
developfrom
fix/refresh-stale-stats-on-inode-rotation

Conversation

@ajslater

@ajslater ajslater commented May 7, 2026

Copy link
Copy Markdown
Owner

Summary

Closes the loop that #735 left open: refresh DB stats when an inode rotates without content changing, so the diff's lookup keyspace stays synchronized with disk reality across Docker bind-mount remounts.

Why

The May 1 fix that stopped flagging every comic modified when inodes rotate (689bbc4) correctly silenced the false-modify storm Docker remounts produced — but it left DB stat rows permanently lying about disk reality afterward. Once a remount rotated the kernel's inode space, every _find_moved_paths call ran against a lookup table that no longer matched any of the inodes the disk would emit.

#735's _is_move_compatible guard suppresses the corruption that mismatch produced (file rows whose path was rewritten to a directory). But suppression isn't repair: legitimate cross-remount renames still degrade to delete+add, and the user loses bookmarks tied to the recreated comic.pk.

This PR refreshes the DB stat in place — bulk_update(fields=["stat"]) only, updated_at is not bumped because the file's content hasn't changed.

Trigger condition

A path P qualifies for refresh when:

  1. P is in data.unchanged (present in both DB and disk snapshots)
  2. P is not in data.modified (mtime and size match disk)
  3. db_snap.inode(P) != disk_snap.inode(P) (inode rotated under us)

That predicate fingerprints the Docker remount case and the rsync-with-timestamps file-replacement case. It produces zero refreshes when inodes are stable, so the common case has no DB writes.

Where in the cycle

_queue_poll_events, between _get_diff and the import-task build:

1. Build DatabaseSnapshot
2. Build DiskSnapshot
3. SnapshotDiff(db, disk)
4. (NEW) _refresh_stale_stats(diff)  — only if not force
5. If diff.is_empty() → return
6. Build ImportTask from diff events → queue

Skipped on force=True polls because force routes every path through the import pipeline (where presave() already rewrites stats), and double-writing serves no purpose.

Implementation notes

  • DatabaseSnapshot now tracks the source model for each path in a new _path_to_model map. Snapshot.model_for_path() exposes it; DiskSnapshot leaves it empty.
  • SnapshotDiff exposes stale_stat_refreshes: tuple[StaleStatRefresh, ...]. Each carries path, model, and the fresh disk os.stat_result.
  • The poller buckets refreshes by model so each affected table takes exactly one bulk_update per cycle.

Operational

  • Zero DB writes on cycles where inodes are stable.
  • One bulk_update per affected model on a remount.
  • Idempotent: a second cycle after a successful refresh sees no inode drift and emits zero refreshes.

Test plan

  • make fix and make lint clean (preexisting remark exit-1 unrelated)
  • pytest tests/ — 48 passed (5 new diff-layer unit tests, 2 new Django integration tests, 41 pre-existing)
  • Run codex on a database that's been corrupted by the original bug; confirm post-restart stats refresh and no further phantom moves are logged
  • Force-restart Docker container so kernel reassigns inodes; confirm next poll emits "Refreshed stale stat on N unchanged-content paths" and updated_at on those rows is unchanged
  • Manual --force poll: confirm the refresh path is skipped (force path handles its own stat updates via presave)

🤖 Generated with Claude Code

The May-1 fix that stopped flagging every comic modified when inodes
rotate (689bbc4) correctly silenced the false-modify storm but left
DB stat rows permanently lying about disk reality afterward — once a
Docker bind-mount remount rotated the kernel's inode space, the
DB's stored inodes no longer pointed at anything on disk, and
``_find_moved_paths`` had a stale lookup table from there on. The
``_is_move_compatible`` guard added in the previous PR suppresses the
corruption that produced, but legitimate cross-remount renames still
degrade to delete+add (and the user loses bookmarks on the recreated
comic.pk).

Phase 3 closes the loop. ``SnapshotDiff`` now emits
``StaleStatRefresh`` payloads for every ``data.unchanged`` path whose
mtime+size still match but whose inode has rotated. The poller writes
those fresh stats back to the appropriate model with one
``bulk_update`` per affected table, with ``fields=["stat"]`` only —
``updated_at`` is intentionally NOT bumped, since the file's content
is unchanged and bookmark "fresh" semantics must hold.

The refresh is skipped on ``force=True`` polls because force routes
every path through the import pipeline (where ``presave`` already
rewrites stats), and we don't want to double-write.

To target the right model with one bulk write per table,
``DatabaseSnapshot`` now tracks the source model for each path in a
new ``_path_to_model`` map; ``Snapshot.model_for_path`` exposes it,
and ``DiskSnapshot`` leaves the map empty.

Operational characteristics:
- Zero DB writes when inodes are stable (the common case). The diff
  loop runs in O(unchanged) time but the per-path inode comparison
  is just a tuple equality.
- One ``bulk_update`` per affected model on a remount (Comic, Folder,
  FailedImport, CustomCover separately).
- Idempotent: a second cycle after a successful refresh sees inodes
  matching disk and emits zero refreshes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ajslater
ajslater merged commit 82be7ce into develop May 7, 2026
3 checks passed
@ajslater
ajslater deleted the fix/refresh-stale-stats-on-inode-rotation branch May 11, 2026 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant