Make a loaded adapter row describe the adapter it actually loaded
The report row contradicted itself. The reason came from the attachment record while path, adapter_id, base_model, origin and cartridge_id came from whatever resolution preferred right now. Reproduced without any stubbing: a live model holds adapter X, training writes a newer adapter Y, and the endpoint then answers state=loaded adapter_id=adapter-Y while explaining "attached to the live model from .../x-attached". A JSON consumer reads the identity columns as the loaded adapter and gets the wrong one. The same root cause had a second symptom. Attachments were keyed by kind with last-writer-wins, so with two live runtimes one unload made the row claim "that model is gone" while the other runtime's model was still alive holding its adapter. LoadRecord now carries the AdapterSource it came from, so a row built from a record has exactly one identity, and the registry is a list of attachments each holding its own weak reference: loaded means some live model still holds an adapter, and when more than one does the reason names the other. clear_loads had no production caller once unload stopped erasing the process record, so it is gone rather than kept as a test-only entry point. Two tests pin both symptoms, and both were mutation-checked: restoring the identity to the resolved source fails the first, and letting live_attachments ignore liveness fails the second and the unload test. pytest -q: 227 passed. ruff clean. mypy cold cache on the three touched source files: 2 errors, byte-identical to the same command on a stashed HEAD tree, net zero. pyright: 2 errors, both the known qwen_vl_utils tuple artifact, unchanged from HEAD.
This commit is contained in:
1 parent
2d1cfdadc7
commit
7ec25bf629
3 files changed
+110
-60
No files matched your search
@@ -34,16 +34,25 @@ class AdapterSource:
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class LoadRecord:
|
||||
"""An adapter this process attached to a model."""
|
||||
"""One adapter this process attached to one model."""
|
||||
|
||||
kind: TrainingAdapterKind
|
||||
adapter_id: str
|
||||
path: str
|
||||
source: AdapterSource
|
||||
sha256: str
|
||||
|
||||
@property
|
||||
def kind(self) -> TrainingAdapterKind:
|
||||
return self.source.manifest.kind
|
||||
|
||||
_LOADED: dict[str, LoadRecord] = {}
|
||||
_LIVE: dict[str, weakref.ReferenceType[object]] = {}
|
||||
@property
|
||||
def adapter_id(self) -> str:
|
||||
return self.source.manifest.adapter_id
|
||||
|
||||
@property
|
||||
def path(self) -> str:
|
||||
return str(self.source.path)
|
||||
|
||||
|
||||
_ATTACHMENTS: list[tuple[LoadRecord, weakref.ReferenceType[object]]] = []
|
||||
|
||||
|
||||
def read_manifest(path: Path) -> AdapterManifest:
|
||||
@@ -153,31 +162,21 @@ def _preference(source: AdapterSource) -> tuple[bool, bool, float]:
|
||||
|
||||
|
||||
def load_records() -> list[LoadRecord]:
|
||||
return list(_LOADED.values())
|
||||
"""Every adapter still attached, or attached since the last load, in attachment order."""
|
||||
return [record for record, _reference in _ATTACHMENTS]
|
||||
|
||||
|
||||
def live_kinds() -> set[str]:
|
||||
"""Kinds whose attached model is still alive, not merely attached once."""
|
||||
alive = {kind for kind, reference in _LIVE.items() if reference() is not None}
|
||||
for kind in [kind for kind, reference in _LIVE.items() if reference() is None]:
|
||||
del _LIVE[kind]
|
||||
return alive
|
||||
|
||||
|
||||
def clear_loads() -> None:
|
||||
_LOADED.clear()
|
||||
_LIVE.clear()
|
||||
def live_attachments() -> list[LoadRecord]:
|
||||
"""Attachments whose model still exists; a dead model cannot be claimed as loaded."""
|
||||
return [record for record, reference in _ATTACHMENTS if reference() is not None]
|
||||
|
||||
|
||||
def record_load(source: AdapterSource, model: Any) -> LoadRecord:
|
||||
record = LoadRecord(
|
||||
kind=source.manifest.kind,
|
||||
adapter_id=source.manifest.adapter_id,
|
||||
path=str(source.path),
|
||||
sha256=source.manifest.sha256,
|
||||
)
|
||||
_LOADED[record.kind.value] = record
|
||||
_LIVE[record.kind.value] = weakref.ref(model)
|
||||
record = LoadRecord(source=source, sha256=source.manifest.sha256)
|
||||
_ATTACHMENTS[:] = [
|
||||
(item, reference) for item, reference in _ATTACHMENTS if reference() is not None
|
||||
]
|
||||
_ATTACHMENTS.append((record, weakref.ref(model)))
|
||||
logger.info("loaded adapter %s (%s) from %s", record.adapter_id, record.kind.value, record.path)
|
||||
return record
|
||||
|
||||
@@ -197,44 +196,57 @@ def plan(settings: Settings, database: Database) -> AdapterLoadReport:
|
||||
sources, mounted = discover(settings, database)
|
||||
rows: list[AdapterChoice] = []
|
||||
winners: dict[str, AdapterSource | None] = {}
|
||||
alive = live_kinds()
|
||||
attached = live_attachments()
|
||||
for kind in TrainingAdapterKind:
|
||||
source, state, reason = resolve(sources, settings, kind)
|
||||
winners[kind.value] = source
|
||||
record = next((item for item in load_records() if item.kind == kind), None)
|
||||
if record is not None:
|
||||
if kind.value in alive:
|
||||
state, reason = "loaded", f"attached to the live model from {record.path}"
|
||||
else:
|
||||
state = "available"
|
||||
reason = f"attached earlier in this process from {record.path}; that model is gone"
|
||||
rows.append(
|
||||
AdapterChoice(
|
||||
kind=kind,
|
||||
state=state,
|
||||
reason=reason,
|
||||
path=str(source.path) if source else None,
|
||||
adapter_id=source.manifest.adapter_id if source else None,
|
||||
base_model=source.manifest.base_model if source else settings.model_id,
|
||||
origin=source.origin if source else None,
|
||||
cartridge_id=source.cartridge_id if source else None,
|
||||
)
|
||||
)
|
||||
live = [record for record in attached if record.kind == kind]
|
||||
earlier = next((record for record in reversed(load_records()) if record.kind == kind), None)
|
||||
if live:
|
||||
newest = live[-1]
|
||||
state = "loaded"
|
||||
reason = f"attached to the live model from {newest.path}"
|
||||
held = sorted({record.adapter_id for record in live[:-1] if record.adapter_id != newest.adapter_id})
|
||||
if held:
|
||||
reason = f"{reason}; another live model still holds {' and '.join(held)}"
|
||||
rows.append(_choice(kind, state, reason, newest.source))
|
||||
continue
|
||||
if earlier is not None and source is not None:
|
||||
state = "available"
|
||||
reason = f"{reason}; {earlier.adapter_id} was attached earlier in this process and that model is gone"
|
||||
rows.append(_choice(kind, state, reason, source))
|
||||
for source in sources:
|
||||
winner = winners.get(source.manifest.kind.value)
|
||||
if winner is None or source.path == winner.path:
|
||||
continue
|
||||
rows.append(
|
||||
AdapterChoice(
|
||||
kind=source.manifest.kind,
|
||||
state="deferred",
|
||||
reason=f"shadowed by {winner.manifest.adapter_id} for the current mount",
|
||||
path=str(source.path),
|
||||
adapter_id=source.manifest.adapter_id,
|
||||
base_model=source.manifest.base_model,
|
||||
origin=source.origin,
|
||||
cartridge_id=source.cartridge_id,
|
||||
_choice(
|
||||
source.manifest.kind,
|
||||
"deferred",
|
||||
f"shadowed by {winner.manifest.adapter_id} for the current mount",
|
||||
source,
|
||||
shadowed=True,
|
||||
)
|
||||
)
|
||||
return AdapterLoadReport(model_id=settings.model_id, mounted=mounted, adapters=rows)
|
||||
|
||||
|
||||
def _choice(
|
||||
kind: TrainingAdapterKind,
|
||||
state: AdapterState,
|
||||
reason: str,
|
||||
source: AdapterSource | None,
|
||||
shadowed: bool = False,
|
||||
) -> AdapterChoice:
|
||||
"""Build one report row from the adapter the row is actually talking about."""
|
||||
return AdapterChoice(
|
||||
kind=kind,
|
||||
state=state,
|
||||
reason=reason,
|
||||
path=str(source.path) if source else None,
|
||||
adapter_id=source.manifest.adapter_id if source else None,
|
||||
base_model=source.manifest.base_model if source else None,
|
||||
origin=source.origin if source else None,
|
||||
cartridge_id=source.cartridge_id if source else None,
|
||||
shadowed=shadowed,
|
||||
)
|
||||
@@ -117,9 +117,9 @@ class _FakeProcessor:
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_stale_loads():
|
||||
adapters.clear_loads()
|
||||
adapters._ATTACHMENTS.clear()
|
||||
yield
|
||||
adapters.clear_loads()
|
||||
adapters._ATTACHMENTS.clear()
|
||||
|
||||
|
||||
def test_a_library_adapter_is_selected_for_its_kind(tmp_path: Path) -> None:
|
||||
@@ -219,6 +219,43 @@ def test_unload_stops_claiming_a_model_that_is_gone(tmp_path: Path, monkeypatch)
|
||||
assert "that model is gone" in row.reason, row.reason
|
||||
|
||||
|
||||
def test_a_loaded_row_names_the_adapter_that_is_actually_attached(tmp_path: Path, monkeypatch) -> None:
|
||||
settings = _settings(tmp_path)
|
||||
attached = _write_adapter(settings.adapters_dir / "x", adapter_id="adapter-x")
|
||||
monkeypatch.setattr(adapters, "load_into_model", lambda model, path: _FakeModel())
|
||||
monkeypatch.setattr(QwenRuntime, "_load_base_model", lambda self: (_FakeModel(), object()))
|
||||
runtime = QwenRuntime(settings)
|
||||
runtime._load_transformers()
|
||||
_write_adapter(settings.adapters_dir / "y", adapter_id="adapter-y")
|
||||
row = next(item for item in adapters.plan(settings, Database(settings)).adapters if item.kind == CORE)
|
||||
assert row.state == "loaded"
|
||||
assert row.adapter_id == "adapter-x", f"the row promises {row.adapter_id} but {attached} is attached"
|
||||
assert row.path == str(attached) and row.reason.endswith(str(attached)), row.reason
|
||||
|
||||
|
||||
def test_one_unload_does_not_declare_another_live_model_gone(tmp_path: Path, monkeypatch) -> None:
|
||||
settings = _settings(tmp_path)
|
||||
_write_adapter(settings.adapters_dir / "x", adapter_id="adapter-x")
|
||||
monkeypatch.setattr(adapters, "load_into_model", lambda model, path: _FakeModel())
|
||||
monkeypatch.setattr(QwenRuntime, "_load_base_model", lambda self: (_FakeModel(), object()))
|
||||
first = QwenRuntime(settings)
|
||||
first._load_transformers()
|
||||
_write_adapter(settings.adapters_dir / "y", adapter_id="adapter-y")
|
||||
second = QwenRuntime(settings)
|
||||
second._load_transformers()
|
||||
database = Database(settings)
|
||||
row = next(item for item in adapters.plan(settings, database).adapters if item.kind == CORE)
|
||||
assert row.state == "loaded" and row.adapter_id == "adapter-y"
|
||||
assert "another live model still holds adapter-x" in row.reason, row.reason
|
||||
first.unload()
|
||||
row = next(item for item in adapters.plan(settings, database).adapters if item.kind == CORE)
|
||||
assert row.state == "loaded" and row.adapter_id == "adapter-y"
|
||||
assert "gone" not in row.reason, row.reason
|
||||
second.unload()
|
||||
row = next(item for item in adapters.plan(settings, database).adapters if item.kind == CORE)
|
||||
assert row.state == "available" and "that model is gone" in row.reason, row.reason
|
||||
|
||||
|
||||
def test_a_rejected_candidate_does_not_claim_another_adapter_won(tmp_path: Path) -> None:
|
||||
settings = _settings(tmp_path)
|
||||
_write_adapter(settings.adapters_dir / "router", kind=TrainingAdapterKind.hme_router, base_model="Qwen/Qwen3.5-0.8B")
|
||||
|
||||
Reference in new issue
Block a user