Trustworthy garbage collection: complete reference discovery (#1469) + race-safe deletion (#1445)

[dimitri-yatsenko]

Umbrella tracking the invariant: garbage collection must never delete an object that is still referenced. Today it can, via two independent failure modes that look similar but have different root causes:

1. Incomplete discovery — Garbage collection deletes external-store files belonging to existing rows (custom codecs) #1469: dj.gc.scan hardcodes the built-in codec names (hash/blob/attach, object/npy), so files referenced by a custom codec are never enumerated. The store scan then reports live files as orphans and collect() deletes them. (Row delete also fails to remove custom-codec files.)
2. Stale discovery / TOCTOU race — Two-phase, transaction-safe garbage collection (quarantine -> grace -> purge) #1445: the scan is correct, but an insert during the scan→delete window has its file deleted out from under it.

They are complementary layers of one goal, not duplicates — and must be fixed in order:

• Garbage collection deletes external-store files belonging to existing rows (custom codecs) #1469 — complete reference discovery (correctness, FIRST). Codec-owned referenced_paths hook; scan and delete-cleanup become codec-driven instead of name-hardcoded. Closes active data loss for real pipelines (aeon_mecha).
• Two-phase, transaction-safe garbage collection (quarantine -> grace -> purge) #1445 — race-safe deletion (hardening, SECOND). Two-phase quarantine → grace → purge with re-check-before-delete; _trash/ prefix for state.

Why the sequence matters

#1445's purge() re-check reruns the scan. On a codec-blind scan (pre-#1469) it would still classify a live custom-codec file as an orphan and purge it after the grace window — so the grace window can't save you from a scan that never sees the reference. Complete discovery (#1469) is the precondition for safe deletion (#1445).

Documentation (datajoint-docs) — to land with the implementation

Add

• reference/specs/garbage-collection.md (new normative spec — none exists today). The orphan-determination model, the referenced_paths codec contract, the two-phase quarantine/grace/purge state machine, config keys (gc.grace_seconds), re-check/concurrency semantics, backend atomic-move requirements, and restore. (Two-phase, transaction-safe garbage collection (quarantine -> grace -> purge) #1445 explicitly asks for a written spec.)

Update

• how-to/garbage-collection.md ("Clean Up Object Storage") — document the two-phase workflow (quarantine / purge / restore, grace_seconds); note custom-codec external files are now handled; revise the "single-pass, best-effort" admonition added in fix issue #186 #189 once two-phase lands.
• reference/specs/codec-api.md — document referenced_paths as part of the Codec contract (required for any codec that owns external artifacts).
• how-to/create-custom-codec.md, explanation/custom-codecs.md, how-to/use-plugin-codecs.md — author guidance: if your codec writes external files, implement referenced_paths so delete + GC see them (otherwise files leak or, worse, get misclassified as orphans and deleted).
• reference/specs/provenance.md — the GC concurrency wording references single-pass semantics; align once two-phase ships (minor).
• Optionally a short explainer (e.g. in explanation/object-storage-overview.md) on how DataJoint tracks external references — codecs own their paths; delete and GC consult them.

[dimitri-yatsenko]

Refinement: two addressing schemes = two GC models; branching safely deferred

Confirmed in code (builtin_codecs/schema.py::_build_path + storage.generate_token): schema-addressed paths are {schema}/{table}/{pk}/{field}{ext} plus a unique per-write token (default length 8). So schema-addressed is not one-file-per-row — the token enables multiple versions of the same logical row's object (immutable writes; a recompute/update lands at a new token path).

That gives two distinct GC models, to be specified separately:

• Hash-addressed (<blob@> / <attach@> / <hash@>) — content-addressed, deduped, many entries → one object. Orphan ⟺ reference count reaches zero across all rows. The harder determination, and where the Two-phase, transaction-safe garbage collection (quarantine -> grace -> purge) #1445 scan→delete race bites hardest.
• Schema-addressed (<object@> / <npy@>) — token-per-write, row-owned versions. Orphan ⟺ a token path no current row references (superseded tokens fall out naturally). Essentially per-row; no cross-entry sharing.

Both are discovered by the same referenced_paths codec hook (#1469): it returns the path(s) the current stored value references, so live tokens are retained and superseded ones become orphans — for both models.

Branching is deliberately deferred until #1469 and #1445 are addressed, and that's safe:

• Schema-addressed's token is globally unique per write → already branching-compatible (two branches computing the same PK write distinct token paths, no collision).
• Hash-addressed is content-addressed/immutable → branch-shareable by nature.
• So branching requires no addressing change; it only widens GC's reference scope later (a hash object is orphaned only when no branch references it), which layers onto this design without invalidating it.

Net: proceed on the current addressing model — #1469 (discovery) → #1445 (two-phase deletion) — and revisit only GC's reference scope when branching lands.

[dimitri-yatsenko]

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.

[MilagrosMarin]

Adding three concrete findings from a v2.3.1 code audit that slot into the umbrella framing (completeness in #1469 territory, race-safety in #1445 territory). Not proposing to replace the two-phase plan — surfacing them so they inform the redesign.

Completeness layer (post-#1469)

Even with codec-driven discovery in place, two paths silently truncate the reference set:

1. _references() swallows per-table exceptions (gc.py:158-178)

try:
    for row in table.proj(attr_name).cursor(as_dict=True):
        try:
            for path, _ in codec.referenced_paths(row[attr_name]):
                referenced.add(path)
        except Exception as e:
            logger.warning(...)  # partial reference set returned
except Exception as e:
    logger.warning(...)  # entire table's refs dropped

Any transient error mid-scan (a Lost connection to MySQL server during query mid-cursor, an intermittent server error, an unexpected exception in codec.referenced_paths) is logged as a warning; the loop moves on and _references() returns a partial set with no completeness signal to the caller. collect(dry_run=False) then computes orphaned = stored - PARTIAL_referenced and deletes those files at gc.py:355-375 — a live file becomes orphaned by a mid-scan hiccup.

Concrete failure: table with 500 hash-referenced rows, connection drops after row 20 → 480 live files land in orphaned_hash_paths → permanently deleted, with one WARNING line in the log.

2. Codec.referenced_paths() returns [] on bad input (codecs.py:252-260)

def referenced_paths(self, stored):
    if isinstance(value, str):
        try:
            value = json.loads(value)
        except (json.JSONDecodeError, TypeError, ValueError):
            return []          # silent zero refs
    if isinstance(value, dict) and "path" in value:
        return [(value["path"], value.get("store"))]
    return []                   # dict without "path" → silent zero refs

Corrupted metadata (a partial-write JSON blob, a JSONB double-encoding, a custom codec whose format drifts) contributes zero references, and its live backing file becomes an orphan candidate. The tests at test_gc.py:117-121, 168-169 enshrine this behavior, so it's intentional — but under dry_run=False it's a silent-data-loss path.

Race-safety layer (#1445 territory)

3. collect() scans references before listing stored files (gc.py:333-340)

Current order:

hash_paths_referenced = self.hash_references()            # line 333
schema_paths_referenced = self.schema_references()        # line 334
for schema in schemas:
    hash_paths_stored.update(self.list_hash_paths(...))   # line 336-340
    schema_paths_stored.update(self.list_schema_paths(...))

A concurrent writer whose put_buffer() + INSERT sequence completes between the reference scan and the stored listing gets its file classified as orphan and deleted — DB row survives pointing at a deleted file. Reversing the order (list stored first, then scan references) narrows the window but doesn't close it (a writer whose upload completes before listing but whose INSERT commits after the reference scan is still exposed) — which is why the full quarantine → grace → purge in #1445 is the durable fix.

Why they interact

The naïve "raise on partial scan" fix for finding #1 collides with finding #2: as it stands, the cursor loop at gc.py:170-176 swallows anything raised by codec.referenced_paths(), so tightening one without the other trades silent data loss for aborted collections. A coordinated fix probably needs:

• Codec.referenced_paths() to distinguish "genuinely no refs" from "couldn't determine" (e.g., a specific exception, a sentinel, or a two-return signature).
• _references() to propagate an incomplete-scan flag to collect(), which refuses to delete when it's set.

Both feed the completeness contract in the umbrella framing.

Not a fix

Docs-only surface; the two-phase redesign in #1445 is a superset. Just flagging that the completeness layer needs its own tightening (findings 1 & 2) even after #1469 lands, and finding 3 is a one-line reorder that could be a stopgap for the race window until the quarantine work merges.

[dimitri-yatsenko]

Progress: the first child — #1469 (complete reference discovery) — is done and merged in #1479 (codec-driven reference discovery + file-level orphan matching; #1479 also closed #1486/#1487). Checked off above.

Remaining before this umbrella closes:

• Two-phase, transaction-safe garbage collection (quarantine -> grace -> purge) #1445 — two-phase quarantine → grace → purge deletion. Its precondition (Garbage collection deletes external-store files belonging to existing rows (custom codecs) #1469) is now satisfied, but the state machine itself is unstarted. (Note: fix(gc): don't act on partial storage listings #1506 is a scan-completeness hardening follow-up, not the two-phase API.)
• The datajoint-docs work — new reference/specs/garbage-collection.md spec, plus the codec-api.md / custom-codec / how-to updates listed above.