ankitects / ankitects/anki

Media DB stores file mtimes in milliseconds but the change tracker compares seconds, so every downloaded media file is rehashed once

Open Beginner friendly
#5,589 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
30.5k
Forks
3.2k
Avg merge
2d 8h
Merged PRs (30d)
60

Description

## Summary

`rslib/src/media/files.rs::mtime_as_i64` returns `.as_millis()`, and that value is written into `collection.media.db2` for files arriving from AnkiWeb (`add_file_from_ankiweb` → `record_additions`) and for `MediaManager::add_file`.

The media folder scan in `rslib/src/sync/media/database/client/changetracker.rs::media_folder_changes` computes the on-disk mtime inline with `.as_secs()` and compares `previous_mtime == mtime`. A millisecond value never equals a second value, so the "mtime unchanged" fast path can never hit for those rows, and every such file falls through to a full `sha1_of_file` read.

## Effect

It is self-healing: the rescan rewrites the row in seconds, so it is one rehash per file, not per sync. But it lands in bulk. After a first sync or any large media download, the next time the media folder mtime changes, the entire downloaded set is read and hashed. On a local SSD that is a few minutes for tens of thousands of files. On a network or FUSE-backed media folder it is hours, and if the process is interrupted nothing is persisted (the whole walk runs in one `transact`), so it restarts from zero next time and media sync never completes.

Observed on Anki 26.08.1 with a 74k-file media folder on network storage: `collection.media.db2` rows all held 13-digit values (`1789116310867`, etc.); the folder scan read every file at ~4 files/s.

## Where it comes from

At tag `2.1.50` both sides used `.as_secs() as i64` (`rslib/src/media/files.rs:318`, `rslib/src/media/changetracker.rs:148`). Commit cf45cbf4 ("Rework syncing code", #2329, 2023-01-18) changed `files.rs` to `.as_millis()` and left the change tracker at `.as_secs()`. Neither line has been touched since; the mismatch is present at `main` and at tag `26.08.1`.

Writers of the `mtime` column today:
- `files.rs:428` (`add_file_from_ankiweb`) → ms
- `media/mod.rs:72` (`MediaManager::add_file`) → ms
- `changetracker.rs:206` (`add_updated_entries`, after a rehash) → seconds

Reader: `changetracker.rs:136-150` → seconds.

## Proposed fix

Make the change tracker call `mtime_as_i64(dentry.path())` instead of inlining `.as_secs()`, so everything is in milliseconds and `mtime_as_i64` is the single source of truth. `folder_mtime` already uses `mtime_as_i64` on both sides (`syncer.rs:175`, `changetracker.rs:59`), so this direction keeps it at ms granularity; going the other way would coarsen it back to 1 s.

Compatibility: rows the change tracker previously wrote in seconds get one transitional rehash on the first scan after upgrade, then everything matches permanently. Rows written by downloads/`add_file` start matching immediately. Tests to update in the same change: `changetracker.rs:297-302` and `:331-336` construct expected entries with `.as_secs() as i64`.

Happy to put up a PR if this direction is acceptable.

Contributor guide

Open the contributing guide

Research direction

Start in rslib/src/sync/media/database/client/changetracker.rs at media_folder_changes and compare its mtime handling with rslib/src/media/files.rs::mtime_as_i64. Review the expected entries in changetracker.rs:297-302 and :331-336, then run the relevant change-tracker tests. Done means downloaded and locally added files use the same mtime representation and the existing expectations pass with the fast path covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases, performance
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.