ITADN

PR #862's DuckDB cache keys on mtime+size, reintroducing the staleness class #844 just closed

#866Openrealmarcin 创建于 9 天前
bug
R
realmarcincommented
Adversarial review of #862. `kg_microbe/query_utils/duckdb_loader.py` decides whether to reuse an existing DuckDB or rebuild it: ```python def _source_fingerprints(nodes_path, edges_path): """Return stable-enough freshness metadata without scanning multi-GB files.""" for kind, path in (("nodes", nodes_path), ("edges", edges_path)): stat = path.stat() result[kind] = (str(path), stat.st_size, stat.st_mtime_ns) ``` This is the timestamp class #844 closed yesterday, in a new place. It has zero references to `kg_microbe/utils/transform_fingerprint.py`, the shared content-fingerprint utility that exists for exactly this decision. ## Two failure directions, one of which is silent **Wasteful but safe:** `git checkout` rewrites the merged TSVs' mtime with no content change (#797), forcing a full rebuild of multi-GB files. Annoying, not wrong. **Silent and wrong:** size plus mtime can both be preserved across a genuine content change. `cp -p`, `rsync --times`, a restore from backup, or an artifact re-extracted from a tarball can all land different bytes with the recorded mtime and an identical size — merged TSVs of similar vintage are very close in size. The database is then judged current and **serves stale query results with no indication**. A wrong answer from `kg query` is materially worse than a slow one. ## The tradeoff is real and I am not asking to hash gigabytes The docstring is candid — "stable-enough … without scanning multi-GB files" — and hashing the merged TSVs on every CLI startup would be unacceptable. But there are cheaper options that do not key on the clock: 1. Have the **merge** write a fingerprint beside its output, the way transforms now do (#844), and have DuckDB record *that*. The merge computes it once; the query path only compares strings. 2. Hash a bounded prefix plus the size and the row count. Still cheap, and defeats the same-size collision. 3. If mtime is kept, record it **alongside** a content signal rather than as the sole one. Related: #845 (nothing detects staleness against another transform's output) is the same family. Worth deciding once where the repo's "has this input changed" answer lives, rather than a third mechanism appearing next.
0 条评论