Skip to content

Commit e5bdb9b

Browse files
refactor(gc): require schema_name; walk only the schema's own sections
Per review, GC is now strictly per-schema and the schema-addressed walk is confined to the schema's own section — a much smaller, safer surface: - list_stored_hashes / list_schema_paths take schema_name as a REQUIRED keyword-only argument (no more whole-store mode). list_schema_paths walks ONLY {schema_prefix}/{schema}/; list_stored_hashes ONLY {hash_prefix}/{schema}/. - Because those two top-level sections are mutually exclusive (store validation) and the filepath section is a third, the schema walk can never enter the hash or filepath sections. So all of this is deleted: * the hash-section skip (and the _hash-substring guard it needed), * the filepath-section skip, * the schema-OR-hash union coverage added for the prefix-change corner — a stranded hash object simply cannot appear in a schema-only walk. Orphan coverage is now schema references alone. - GC's blast radius shrinks to exactly {hash_prefix}/{schema}/ and {schema_prefix}/{schema}/: user <filepath@> content and other schemas' objects are structurally out of reach. Resolves the cohabitation hazard for the common case without a separate fail-safe. - collect() builds bytes_freed size maps per schema (guarded by orphan counts so the mocked unit tests don't invoke real listers). Legacy note: 2.3.0-and-earlier stores wrote schema-addressed objects at root-level {schema}/... (schema_prefix ignored, #1487). Such a store should set schema_prefix='' so writes and GC both use root-level; the per-schema walk then targets {schema}/ directly. Otherwise those objects stay readable but outside the scanned section. Tests updated to pass schema_name; the filepath test now asserts the hazard is gone by construction; the _hash-substring test keeps its positive assertion. GC suite 39/39; hash/object/npy/chaining 96 passed.
1 parent b0dad04 commit e5bdb9b

2 files changed

Lines changed: 120 additions & 155 deletions

File tree

src/datajoint/gc.py

Lines changed: 88 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@
3131
# Remove orphaned items (dry_run=False to actually delete)
3232
stats = dj.gc.collect(schema1, schema2, store_name='mystore', dry_run=True)
3333
34+
Both sections embed the schema name in every path, so garbage collection is
35+
**per-schema**: each schema is scanned against its own subtree only. Orphan
36+
detection is therefore confined to the schemas you pass — any subset of the
37+
schemas sharing a store may be scanned/collected safely, and GC never touches
38+
another schema's objects or user-managed content elsewhere in the store.
39+
3440
See Also
3541
--------
3642
datajoint.builtin_codecs : Codec implementations for object storage types.
@@ -255,32 +261,29 @@ def scan_schema_references(
255261
return referenced
256262

257263

258-
def list_stored_hashes(store_name: str | None = None, config=None, schema_name: str | None = None) -> dict[str, int]:
264+
def list_stored_hashes(store_name: str | None = None, config=None, *, schema_name: str) -> dict[str, int]:
259265
"""
260-
List hash-addressed items in storage.
266+
List a schema's hash-addressed items in storage.
261267
262-
Scans the store's configured hash-addressed section — the ``hash_prefix``
263-
setting, default ``_hash/`` — and returns the stored objects found. These
264-
correspond to ``<hash@>``, ``<blob@>``, and ``<attach@>`` types. Only the
265-
CURRENTLY configured prefix is walked: objects written under a former
266-
prefix are not listed here (they remain readable via their metadata paths,
267-
and :func:`scan` still protects them from deletion — see its note).
268+
Walks exactly one schema's subtree of the configured hash-addressed
269+
section — ``{hash_prefix}/{schema_name}/`` (``hash_prefix`` default
270+
``_hash``) — and returns the stored objects (``<hash@>``, ``<blob@>``,
271+
``<attach@>``). Every hash path embeds the schema and deduplication is
272+
per-schema, so this listing is complete for that schema and independent of
273+
every other schema on the store.
268274
269-
When ``schema_name`` is given, the walk is confined to that schema's
270-
subtree (``{hash_prefix}/{schema}/``). Because every hash path embeds the
271-
schema and deduplication is per-schema, this is complete for that schema —
272-
which is what lets :func:`scan` operate on one schema without seeing (or
273-
endangering) objects belonging to other schemas sharing the store.
275+
Only the CURRENTLY configured prefix is walked: objects written under a
276+
former prefix are not listed here (they remain readable via their metadata
277+
paths). ``schema_name`` is required — GC is always per-schema.
274278
275279
Parameters
276280
----------
277281
store_name : str, optional
278282
Store to scan (None = default store).
279283
config : Config, optional
280284
Config instance. If None, falls back to global settings.config.
281-
schema_name : str, optional
282-
Restrict the walk to this schema's subtree. None = the whole hash
283-
section (all schemas).
285+
schema_name : str
286+
Schema whose hash subtree to list (required, keyword-only).
284287
285288
Returns
286289
-------
@@ -299,14 +302,10 @@ def list_stored_hashes(store_name: str | None = None, config=None, schema_name:
299302
config = _global_config
300303
# Hash-addressed storage: {hash_prefix}/{schema}/{subfolders...}/{hash}.
301304
# The prefix comes from the store's settings — the same value the writer
302-
# uses — so scanner and writer cannot drift. NOTE: only the CURRENTLY
303-
# configured prefix is scanned; objects written under a previous prefix
304-
# value remain readable (their metadata stores full paths) but are not
305-
# candidates for reclamation until the setting is restored.
305+
# uses — so scanner and writer cannot drift.
306306
_spec = config.get_store_spec(store_name)
307307
hash_prefix = _spec["hash_prefix"].strip("/") + "/" # settings applies the "_hash" default
308-
# Confine the walk to one schema's subtree when requested.
309-
section = f"{hash_prefix}{schema_name}/" if schema_name else hash_prefix
308+
section = f"{hash_prefix}{schema_name}/"
310309
# Base32 pattern: 26 lowercase alphanumeric chars
311310
base32_pattern = re.compile(r"^[a-z2-7]{26}$")
312311

@@ -343,44 +342,44 @@ def list_stored_hashes(store_name: str | None = None, config=None, schema_name:
343342
return stored
344343

345344

346-
def list_schema_paths(store_name: str | None = None, config=None, schema_name: str | None = None) -> dict[str, int]:
345+
def list_schema_paths(store_name: str | None = None, config=None, *, schema_name: str) -> dict[str, int]:
347346
"""
348-
List schema-addressed object files in storage.
349-
350-
Enumerates the individual **files** written by schema-addressed codecs.
351-
A single-file object is one file at
352-
``{schema_prefix}/{schema}/{table}/{pk}/{field}_{token}[.ext]``
353-
(``schema_prefix`` defaults to ``_schema``); a directory-valued object
354-
(e.g. a Zarr store) is many files under that path prefix, plus a
355-
``{path}.manifest.json`` sidecar beside it — all are listed. Orphan
356-
detection matches these files against each row's referenced object path
357-
via :func:`_is_covered` (exact, ancestor-prefix, or manifest-sidecar), so
358-
superseded per-token versions are reclaimable while every file belonging
347+
List a schema's schema-addressed object files in storage.
348+
349+
Walks exactly one schema's section — ``{schema_prefix}/{schema_name}/``
350+
(``schema_prefix`` default ``_schema``) — and enumerates the individual
351+
**files** written by schema-addressed codecs. A single-file object is one
352+
file at ``{schema_prefix}/{schema}/{table}/{pk}/{field}_{token}[.ext]``; a
353+
directory-valued object (e.g. a Zarr store) is many files under that path
354+
prefix, plus a ``{path}.manifest.json`` sidecar beside it — all are listed.
355+
Orphan detection matches these files against each row's referenced object
356+
path via :func:`_is_covered` (exact, ancestor-prefix, or manifest-sidecar),
357+
so superseded per-token versions are reclaimable while every file belonging
359358
to a live object — including its manifest — is kept.
360359
361-
Scope: with ``schema_name`` given, the walk is confined to that schema's
362-
two possible locations — the ``{schema_prefix}/{schema}/`` section and the
363-
legacy root-level ``{schema}/`` layout (DataJoint 2.3.0 and earlier). Both
364-
begin with the schema name, so every schema-addressed object lives under
365-
one of them and the per-schema walk is complete. With ``schema_name=None``
366-
the walk covers the WHOLE store (minus the configured hash and filepath
367-
sections) — every schema at once.
368-
369-
In both modes the hash section is skipped, but a hash object can still
370-
surface here when it sits *outside* the current hash section (e.g. after
371-
``hash_prefix`` was changed on a populated store, or when ``hash_prefix``
372-
is empty so hash and schema share the root). :func:`scan` distinguishes
373-
such live objects from true orphans by checking coverage against BOTH the
374-
schema and hash reference sets.
360+
The walk is confined to the schema's own section, so it never enters the
361+
hash section or the filepath section (mutually-exclusive top-level
362+
prefixes, enforced by store validation) — GC's blast radius is exactly
363+
``{schema_prefix}/{schema}/``, and user-managed ``<filepath@>`` content and
364+
other schemas' objects are structurally out of reach.
365+
366+
``schema_name`` is required — GC is always per-schema.
367+
368+
Legacy layout: DataJoint 2.3.0 and earlier wrote schema-addressed objects
369+
at root level ``{schema}/...`` (``schema_prefix`` was ignored — see
370+
datajoint/datajoint-python#1487). Such a store should set
371+
``schema_prefix=""`` so both writes and this walk use the root-level
372+
layout consistently; otherwise those objects remain readable (metadata
373+
records full paths) but are outside the scanned section.
375374
376375
Parameters
377376
----------
378377
store_name : str, optional
379378
Store to scan (None = default store).
380379
config : Config, optional
381380
Config instance. If None, falls back to global settings.config.
382-
schema_name : str, optional
383-
Restrict the walk to this schema's subtree(s). None = whole store.
381+
schema_name : str
382+
Schema whose section to list (required, keyword-only).
384383
385384
Returns
386385
-------
@@ -390,72 +389,32 @@ def list_schema_paths(store_name: str | None = None, config=None, schema_name: s
390389
backend = get_store_backend(store_name, config=config)
391390
stored: dict[str, int] = {}
392391

393-
# A store may declare a `filepath_prefix` — the namespace reserved for
394-
# user-managed <filepath@> content. Files under it are NOT DataJoint-owned
395-
# objects and must never be candidates for garbage collection. (Without a
396-
# declared filepath_prefix, cohabiting filepath files are indistinguishable
397-
# from orphans — see the fail-safe planned with the two-phase GC work.)
398392
if config is None:
399393
from .settings import config as _global_config
400394

401395
config = _global_config
402396
_spec = config.get_store_spec(store_name)
403-
_fp = _spec.get("filepath_prefix")
404-
filepath_prefix = (_fp.strip("/") + "/") if _fp else None
405-
_hp = _spec["hash_prefix"].strip("/") # settings applies the "_hash" default
406-
hash_section = _hp + "/" if _hp else None
407397
_sp = _spec["schema_prefix"].strip("/") # settings applies the "_schema" default
408-
398+
rel = f"{_sp}/{schema_name}" if _sp else schema_name
409399
full_root = backend._full_path("")
410400

411-
# Determine which subtree(s) to walk. Per-schema scoping confines orphan
412-
# detection to the requested schema's own folders — objects belonging to
413-
# other schemas sharing the store are never seen, hence never at risk.
414-
if schema_name:
415-
rel_roots = [f"{_sp}/{schema_name}"] if _sp else []
416-
rel_roots.append(schema_name) # legacy root-level layout (no schema_prefix)
417-
else:
418-
rel_roots = [""] # whole store
419-
420-
for rel in rel_roots:
421-
try:
422-
for root, dirs, files in backend.fs.walk(backend._full_path(rel)):
423-
# Skip the hash-addressed storage subtree. Match on the FIRST
424-
# store-relative path segment only — a substring test would also
425-
# exclude user tables/schemas whose names merely contain "_hash"
426-
# (e.g. a table `probe_hash`), silently leaking their orphans.
427-
rel_root = root.replace(full_root, "").lstrip("/")
428-
if hash_section and (rel_root + "/").startswith(hash_section):
429-
continue
430-
431-
# Skip the declared filepath namespace (user-managed files).
432-
if filepath_prefix and (rel_root + "/").startswith(filepath_prefix):
433-
continue
434-
435-
for filename in files:
436-
# Manifest sidecars ({object}.manifest.json, written for
437-
# folder-valued objects) are deliberately INCLUDED: they are
438-
# owned by their object and must be reclaimed with it when the
439-
# object is orphaned. _is_covered() keeps a live object's
440-
# manifest out of the orphan set.
441-
file_path = f"{root}/{filename}"
442-
relative_path = file_path.replace(full_root, "").lstrip("/")
443-
444-
# Floor guard: a real object is at least {schema}/{table}/.../{file}
445-
# (root-level legacy layout) or {schema_prefix}/{schema}/... —
446-
# both have >= 2 slashes. Drops stray shallow files.
447-
if relative_path.count("/") < 2:
448-
continue
449-
450-
try:
451-
stored[relative_path] = backend.fs.size(file_path)
452-
except Exception:
453-
stored[relative_path] = 0
454-
455-
except FileNotFoundError:
456-
continue # this subtree does not exist (e.g. no legacy layout)
457-
except Exception as e:
458-
logger.warning(f"Error listing stored schemas: {e}")
401+
try:
402+
for root, dirs, files in backend.fs.walk(backend._full_path(rel)):
403+
for filename in files:
404+
# Manifest sidecars ({object}.manifest.json) are INCLUDED: they
405+
# are owned by their object and must be reclaimed with it when
406+
# the object is orphaned. _is_covered() keeps a live object's
407+
# manifest out of the orphan set.
408+
file_path = f"{root}/{filename}"
409+
relative_path = file_path.replace(full_root, "").lstrip("/")
410+
try:
411+
stored[relative_path] = backend.fs.size(file_path)
412+
except Exception:
413+
stored[relative_path] = 0
414+
except FileNotFoundError:
415+
pass # this schema's section does not exist yet
416+
except Exception as e:
417+
logger.warning(f"Error listing stored schemas: {e}")
459418

460419
return stored
461420

@@ -558,14 +517,13 @@ def scan(
558517
listed and never at risk. (This removes the former requirement to pass
559518
*every* schema on a store at once.)
560519
561-
A stored file is considered live if it is covered by EITHER reference set
562-
(schema or hash) for its schema. This keeps a live hash object safe when it
563-
sits outside the current hash section — e.g. after ``hash_prefix`` was
564-
changed on a populated store, or when ``hash_prefix`` is empty so hash and
565-
schema share the root: it is covered by the hash references (its metadata
566-
path), so it is never misclassified as an orphan. The trade-off (documented,
567-
not a bug): objects under a *former* prefix are not reclamation candidates
568-
until the setting is restored.
520+
Each schema's two sections are walked in isolation
521+
(``{hash_prefix}/{schema}/`` and ``{schema_prefix}/{schema}/``), so GC's
522+
blast radius is exactly those folders: user ``<filepath@>`` content and
523+
other schemas' objects are structurally out of reach. Objects under a
524+
*former* prefix (after a prefix change on a populated store) stay readable
525+
via their metadata paths but are not reclamation candidates until the
526+
setting is restored.
569527
570528
Parameters
571529
----------
@@ -620,17 +578,16 @@ def scan(
620578
h_stored = list_stored_hashes(store_name, config=_config, schema_name=db)
621579
orphaned_hashes |= set(h_stored.keys()) - h_ref
622580

623-
# --- Schema-addressed storage (this schema's subtree) ---
581+
# --- Schema-addressed storage (this schema's section) ---
624582
s_ref = scan_schema_references(schema, store_name=store_name, verbose=verbose)
625583
s_stored = list_schema_paths(store_name, config=_config, schema_name=db)
626584
# Coverage, not exact set difference: a referenced path may be a
627585
# DIRECTORY-valued object (e.g. a Zarr store), whose stored form is
628-
# many files under that prefix, plus a `.manifest.json` sidecar.
629-
# A stored file is live if covered by EITHER of THIS schema's reference
630-
# sets — the hash arm protects a live hash object that surfaces in the
631-
# schema walk (empty hash_prefix, or a former prefix under this schema).
632-
live = s_ref | h_ref
633-
orphaned_paths |= {p for p in s_stored if not _is_covered(p, live)}
586+
# many files under that prefix, plus a `.manifest.json` sidecar. The
587+
# schema walk is confined to this schema's schema_prefix section, so it
588+
# can only contain this schema's schema-addressed objects — coverage
589+
# against s_ref alone is complete (no hash objects can appear here).
590+
orphaned_paths |= {p for p in s_stored if not _is_covered(p, s_ref)}
634591

635592
hash_referenced |= h_ref
636593
hash_stored.update(h_stored)
@@ -715,7 +672,11 @@ def collect(
715672
if not dry_run:
716673
# Delete orphaned hashes
717674
if stats["hash_orphaned"] > 0:
718-
hash_stored = list_stored_hashes(store_name, config=_config)
675+
# Size map for bytes_freed — built per schema (listers are
676+
# per-schema) and merged across the passed schemas.
677+
hash_stored: dict[str, int] = {}
678+
for schema in schemas:
679+
hash_stored.update(list_stored_hashes(store_name, config=_config, schema_name=schema.database))
719680

720681
for path in stats["orphaned_hashes"]:
721682
try:
@@ -731,7 +692,9 @@ def collect(
731692

732693
# Delete orphaned schema paths
733694
if stats["schema_paths_orphaned"] > 0:
734-
schema_paths_stored = list_schema_paths(store_name, config=_config)
695+
schema_paths_stored: dict[str, int] = {}
696+
for schema in schemas:
697+
schema_paths_stored.update(list_schema_paths(store_name, config=_config, schema_name=schema.database))
735698

736699
for path in stats["orphaned_paths"]:
737700
try:

0 commit comments

Comments
 (0)