Garbage collection deletes external-store files belonging to existing rows (custom codecs) #1469

Activity

1. dimitri-yatsenko commented on Jul 2, 2026

   

   dimitri-yatsenko

   on Jul 2, 2026

   Member

   More actions

   Root cause (confirmed in gc.py)

   dj.gc.scan enumerates references by hardcoding the built-in codec names: _uses_hash_storage matches hash/blob/attach, _uses_schema_addressed_storage matches object/npy, and _extract_hash_refs / _extract_schema_path_refs pull the paths out of those columns. A custom SchemaCodec subclass has none of those names, so its stored files are never enumerated as references. The store scan then finds the .nc files, matches them against an (empty) reference set for that column, and reports live files as orphans — so collect() deletes them. The same built-in-only assumption is why row delete never removes the codec's file.

   So both symptoms are one bug: reference discovery is incomplete — GC and delete only understand built-in codecs.

   Proposed fix: a codec-owned referenced_paths hook

   Make reference enumeration codec-driven instead of name-switched. Add an optional method to the Codec contract:

   class Codec:
       def referenced_paths(self, stored_value) -> list[tuple[str, str | None]]:
           """External store paths this codec's stored value references, as
           (path, store_name). Default: no external artifacts."""
           return []

   • Built-in hash/blob/attach and object/npy override it, relocating today's _extract_hash_refs / _extract_schema_path_refs logic into the codecs themselves.
   • Custom codecs that write external files (the NetCDF one here) implement it to return their .nc path(s).
   • dj.gc.scan iterates codec-typed columns and calls codec.referenced_paths(value) per stored value — no codec.name switch. The reference set is then complete for any codec.
   • The same hook drives delete-time cleanup: on row delete, remove the paths its codecs reference (fixes the "orphan left on delete" half of this issue).

   Net: delete removes the codec's file, and GC never classifies a referenced custom-codec file as an orphan.

   Relationship to #1445 (not a duplicate)

   #1445 (two-phase quarantine/grace/purge) addresses the concurrency race — correct scan, stale by delete time. It does not fix this issue: #1445's re-check-before-purge reruns the same scan, so a codec-blind scan would still purge the live file after the grace window. This issue must land first — #1445's re-check is only sound once the scan sees every reference. Both tracked under #1478.

   Priority: active data loss for a real pipeline (aeon_mecha) on 2.x — should be fixed ahead of the #1445 hardening.
2. dimitri-yatsenko commented on Jul 2, 2026

   

   dimitri-yatsenko

   on Jul 2, 2026

   Member

   More actions

   Implementation plan: codec-owned reference discovery

   Concrete plan following the root-cause comment above. Fixes discovery for any codec (not just built-ins) and hardens GC against future codec-blindness. Tracked under #1478.

   1. The codec hook (codecs.py)

   One method on the Codec contract, operating on the stored (encoded) representation already in the DB column — so discovery stays metadata-only (no download/decode):

   class Codec:
       def referenced_paths(self, stored_value) -> list[tuple[str, str | None]]:
           """External store paths (path, store_name) this stored value references.
           Default: none. Any codec that writes to an external store must return them."""
           return []

   2. Built-in implementations (relocate today's logic into the codecs)

   • Hash family (hash/blob/attach): return the hash object path from the stored metadata — move _extract_hash_refs here.
   • Schema family: implement referenced_paths on the SchemaCodec base (builtin_codecs/schema.py), returning the token path from the stored metadata — move _extract_schema_path_refs here. object/npy inherit it.

   Key payoff: a custom codec that subclasses SchemaCodec (exactly the NetCDF case in this issue) inherits a correct referenced_paths for free → this issue is fixed with zero author effort, while fully-custom codecs can still override.

   3. GC scan rewiring (gc.py)

   Replace the codec-name switch (_uses_hash_storage / _uses_schema_addressed_storage) with codec dispatch:

   • For every codec-typed, store-backed column, build the referenced set as the union of attr.codec.referenced_paths(value) over its stored values.
   • orphaned = stored_files − referenced_set. This single union covers both GC models: a hash object referenced by any row stays (reference-count-to-zero), and schema-addressed superseded tokens fall out naturally (row-owned versions).

   4. Fail-safe principle (defense in depth)

   The deeper bug is that GC deletes by exclusion — anything not in a built-in referenced set. Make GC never delete a file it cannot attribute: only purge files within a path-space owned by a codec that participated in referenced_paths; leave unattributable files untouched and warn. With the hook, custom codecs become attributable; this prevents the next codec-blindness from causing data loss.

   5. Delete-time cleanup — intentionally unchanged

   DataJoint's model is delete-then-GC (external files survive row delete by design — dedup + transaction safety; documented in garbage-collection.md). So "delete leaves the file" is working as intended; the actual bug is GC deleting live files. Once discovery is correct, the normal delete → gc.collect() workflow reclaims the custom codec's orphans correctly. Eager delete-time removal stays out of scope (unsafe for hash-dedup).

   6. Tests

   • Unit: referenced_paths for each built-in codec + a custom SchemaCodec subclass (inherited) + a fully-custom override.
   • Regression (this issue's repro): custom SchemaCodec on a file store → insert N rows, delete a subset, gc.scan reports only the deleted rows' files as orphaned; gc.collect(dry_run=False) retains the live files.
   • Hash-dedup: two rows, identical content → one object; delete one row → object still referenced, not orphaned.
   • Schema-token versioning: re-write a row's object → old token becomes orphan, current token retained.

   7. Back-compat & docs

   • Additive method with a safe default → non-breaking.
   • Docs (per Trustworthy garbage collection: complete reference discovery (#1469) + race-safe deletion (#1445) #1478): add referenced_paths to reference/specs/codec-api.md; update create-custom-codec.md / explanation/custom-codecs.md — "if your codec owns external files, implement referenced_paths (or subclass SchemaCodec to inherit it)."

   Sequencing

   Lands as this issue (complete discovery). #1445 (two-phase quarantine/grace/purge) builds on the now-trustworthy scan.
3. dimitri-yatsenko commented on Jul 2, 2026

   

   dimitri-yatsenko

   on Jul 2, 2026

   Last edited by dimitri-yatsenko

   Member

   More actions

   Heads-up while implementing this (PR #1479): the fix also closes a latent data-loss path in built-in schema-addressed storage — <object@> and <npy@>, not just custom codecs.

   A second root cause (in addition to the codec-name blindness): a row's stored metadata references the object file ({schema}/{table}/{pk}/{field}_{token}), but list_schema_paths enumerated the enclosing directory. The referenced and stored path sets therefore never matched, so every live schema-addressed object was classified as an orphan — meaning gc.collect() could delete live <object@>/<npy@> files. It stayed hidden because the existing scan tests only asserted referenced >= 1, never orphaned == 0.

   PR #1479 fixes it by enumerating files (matching the referenced paths, with per-token granularity) and deleting the single orphaned file (+ pruning empty parents). Net: this broadens the severity — any pipeline that ran gc.collect() over schema-addressed columns was exposed, not only custom-codec users.
4. dimitri-yatsenko

   added a commit that references this issue on Jul 6, 2026

   refactor(heading): storage-model classification lives on Heading/Attr…

   ...

   06c9269
5. dimitri-yatsenko

   closed this as completedin #1479on Jul 14, 2026
6. dimitri-yatsenko

   added a commit that references this issue on Jul 14, 2026

   Merge pull request #1479 from datajoint/fix/1469-codec-referenced-paths

   ...

   467ea0f