Skip to content

Commit d0084de

Browse files
committed
fix: recover from ontoenv's stale interrupted-mutation marker
A persistent ontology cache could be opened exactly once. The second open raised CatalogRecoveryError and there is no way back: ontoenv exposes no recovery API, so the cache was permanently unusable. Root cause, traced by wrapping each OntoEnv method and watching the marker file: ontoenv writes .ontoenv/catalog.pending when a batched mutation begins and removes it only when the batch finishes cleanly (BatchScope::run in lib/src/api.rs; its Drop impl does not remove it either). An unresolvable owl:imports makes the batch end in error, so the marker survives. That is the normal case here, not an edge case. Loading Brick calls import_dependencies, which raises ValueError("Failed to resolve graph for URI") for each of eight imports Brick declares and that do not resolve offline -- brickschema 1.3, qudt unit and quantitykind, ashrae bacnet/2020, Brick/ref, rec/brickpatches, rec/recimports, datashapes dash. Several 404 even online. BuildingMOTIF tolerates missing imports and swallows those errors, so the load "succeeds" while the catalog is left marked. An ontology declaring no imports leaves no marker, which is how this was isolated. So `connect` now clears a marker it finds and retries once, with a warning naming the file. This is a workaround for an upstream defect and is marked as such: it trades away the marker's value after a genuine crash mid-write, which is the reason it warns rather than doing it silently, and only retries once. Verified: load Brick into a persistent cache, close, reopen (warns, succeeds, ontology intact), close, open a third time cleanly. The principled fix belongs upstream -- a tolerable non-strict import failure should not leave an interrupted-mutation marker, since non-strict mode is documented as best-effort. Failing that, ontoenv should expose a recovery call so consumers do not have to delete files it owns.
1 parent 390c0c2 commit d0084de

1 file changed

Lines changed: 46 additions & 2 deletions

File tree

buildingmotif/ontology_environment.py

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
import logging
12
from pathlib import Path
23
from typing import TYPE_CHECKING, Any, Dict, Iterable, Optional, Tuple, Union
34

45
import rdflib
5-
from ontoenv import OntoEnv
6+
from ontoenv import CatalogRecoveryError, OntoEnv
67

78
from buildingmotif.database.graph_connection import _is_uuid
89

@@ -57,7 +58,50 @@ def __init__(
5758
# what init_from_store did at construction time
5859
self.env.refresh_from_store(full=True)
5960
else:
60-
self.env = OntoEnv.connect(str(path), graph_store=store, **options)
61+
self.env = self._connect_recovering(str(path), store, options)
62+
63+
@staticmethod
64+
def _connect_recovering(
65+
path: str, store: Optional["BuildingMOTIFGraphStore"], options: Dict[str, Any]
66+
) -> OntoEnv:
67+
"""``OntoEnv.connect``, clearing a stale interrupted-mutation marker.
68+
69+
**This is a workaround for an ontoenv defect; delete it once ontoenv
70+
stops leaving the marker for tolerable failures.**
71+
72+
ontoenv writes ``.ontoenv/catalog.pending`` when it begins a batched
73+
mutation and removes it only when the batch finishes cleanly
74+
(``BatchScope::run``). An *unresolvable* ``owl:imports`` makes the batch
75+
end in error, so the marker survives -- and every later open of that
76+
cache raises ``CatalogRecoveryError``.
77+
78+
That is routine here rather than exceptional: Brick declares eight
79+
imports that do not resolve offline (several 404 even online), so a
80+
single ``Library.from_ontology(Brick)`` permanently poisons a
81+
persistent cache. An ontology with no imports leaves no marker, which
82+
is how this was isolated. ontoenv exposes no recovery call, so the only
83+
way through is to remove the file.
84+
85+
The cost of doing so: after a genuine crash mid-write the marker is a
86+
real signal, and this clears it. That is the trade being made -- one
87+
warned retry, and if the retry also fails the error propagates.
88+
"""
89+
try:
90+
return OntoEnv.connect(path, graph_store=store, **options)
91+
except CatalogRecoveryError:
92+
marker = Path(path) / ".ontoenv" / "catalog.pending"
93+
if not marker.exists():
94+
raise
95+
logging.getLogger(__name__).warning(
96+
"Clearing stale ontoenv recovery marker at %s. This is normally "
97+
"left behind by an unresolvable owl:imports rather than by an "
98+
"actual interrupted write; if this repeats, the ontology cache "
99+
"at %s may genuinely be damaged and can be deleted.",
100+
marker,
101+
path,
102+
)
103+
marker.unlink()
104+
return OntoEnv.connect(path, graph_store=store, **options)
61105

62106
def close(self) -> None:
63107
self.env.close()

0 commit comments

Comments
 (0)