Recover from worker death and deny metadata writes
A crashed worker used to poison the rest of the run. execute() reported the crash as a failed result, but kept the dead session, and every later call then raised RuntimeError out of the tool. Clear the session so the next call checks out a replacement. Keep the session for a syntax or runtime error, which leaves the worker healthy, and say in the failure text that a restart loses the variables. A MemoryFile accepts writes, so metadata.json took them while the other three document files refused. Mount it through the same read and deny pair. The write-denial test now covers all four files.
This commit is contained in:
parent
522959d9b4
commit
94befc0dfd
5 changed files with 118 additions and 89 deletions
|
|
@ -22,6 +22,8 @@
|
|||
- `docs/installation.md` documents the `jina`, `s3` and `ingester` extras, names the extras the full package actually pulls, drops the removed MixedBread AI reranker, and no longer lists Anthropic as a built-in provider.
|
||||
- `docs/tuning.md` no longer points at the removed `claim_timeout_s` setting.
|
||||
- `analysis.code_timeout` is checked before each document read; code that reads in a loop no longer overruns it by the duration of the outstanding reads.
|
||||
- A crashed Monty worker no longer poisons the analysis sandbox for the rest of the run: `execute` reports the crash and the next call checks out a replacement session.
|
||||
- `metadata.json` in the document VFS rejects writes, matching `content.txt`, `items.jsonl` and `toc.json`.
|
||||
|
||||
### Removed
|
||||
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import asyncio
|
|||
import json
|
||||
import os
|
||||
from collections.abc import AsyncIterator, Callable, Coroutine
|
||||
from contextlib import asynccontextmanager
|
||||
from contextlib import asynccontextmanager, suppress
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING, Any, Literal
|
||||
|
|
@ -220,6 +220,19 @@ class Sandbox:
|
|||
)
|
||||
return asyncio.run_coroutine_threadsafe(coro, self._loop).result()
|
||||
|
||||
async def _discard_session(self) -> None:
|
||||
"""Drop a session whose worker is gone.
|
||||
|
||||
The session object is unusable once its worker dies: it answers every
|
||||
later call with ``RuntimeError: this checkout has already been
|
||||
finished``. Clearing it makes ``_ensure_initialized`` check out a
|
||||
replacement, at the cost of the variables the dead worker held.
|
||||
"""
|
||||
session, self._session = self._session, None
|
||||
if session is not None:
|
||||
with suppress(Exception):
|
||||
await session.__aexit__(None, None, None)
|
||||
|
||||
async def close(self) -> None:
|
||||
"""Return the worker to the pool and shut the pool down. Idempotent."""
|
||||
if self._session is not None:
|
||||
|
|
@ -402,7 +415,25 @@ class Sandbox:
|
|||
},
|
||||
ensure_ascii=False,
|
||||
)
|
||||
files.append(MemoryFile(f"{doc_dir}/metadata.json", metadata))
|
||||
|
||||
# A MemoryFile accepts writes, so mount metadata.json through the
|
||||
# same read/deny pair as the rest. The content is already built,
|
||||
# so the reader stays eager.
|
||||
def _make_metadata_reader(
|
||||
text: str,
|
||||
) -> Callable[["PurePosixPath"], str]:
|
||||
def read_metadata(_path: "PurePosixPath") -> str:
|
||||
return text
|
||||
|
||||
return read_metadata
|
||||
|
||||
files.append(
|
||||
CallbackFile(
|
||||
f"{doc_dir}/metadata.json",
|
||||
read=_make_metadata_reader(metadata),
|
||||
write=_deny_write,
|
||||
)
|
||||
)
|
||||
|
||||
def _make_content_reader(
|
||||
did: str,
|
||||
|
|
@ -497,11 +528,20 @@ class Sandbox:
|
|||
print_callback=print_callback,
|
||||
os=vfs,
|
||||
)
|
||||
except pydantic_monty.MontyError as e:
|
||||
except (pydantic_monty.MontyError, RuntimeError) as e:
|
||||
stdout = "".join(stdout_lines)
|
||||
if len(stdout) > max_chars:
|
||||
stdout = stdout[:max_chars] + "\n... (output truncated)"
|
||||
return SandboxResult(stdout=stdout, stderr=str(e), success=False)
|
||||
stderr = str(e)
|
||||
# A crash kills the worker, and a protocol error leaves it out of
|
||||
# step. Both poison the session. Bad user code does not.
|
||||
if isinstance(e, pydantic_monty.MontyCrashedError | RuntimeError):
|
||||
await self._discard_session()
|
||||
stderr = (
|
||||
f"{stderr}\n\nThe interpreter restarted. Variables from "
|
||||
"earlier calls are gone."
|
||||
)
|
||||
return SandboxResult(stdout=stdout, stderr=stderr, success=False)
|
||||
|
||||
stdout = "".join(stdout_lines)
|
||||
if output is not None:
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -430,71 +430,44 @@ class TestSandboxVFS:
|
|||
assert result.stdout.count("True") == 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.vcr()
|
||||
async def test_open_write_denied(self, temp_db_path):
|
||||
"""Opening a document file for writing raises PermissionError."""
|
||||
config = AppConfig()
|
||||
async with HaikuRAG(temp_db_path, create=True) as client:
|
||||
doc = await client.create_document(
|
||||
content="Content about foxes and dogs.",
|
||||
uri="test://doc",
|
||||
title="Fox Document",
|
||||
)
|
||||
|
||||
context = AnalysisContext()
|
||||
sb = Sandbox(db_path=temp_db_path, config=config, context=context)
|
||||
result = await sb.execute(
|
||||
"try:\n"
|
||||
f" with open('/documents/{doc.id}/content.txt', 'w') as f:\n"
|
||||
" f.write('nope')\n"
|
||||
" print('WROTE')\n"
|
||||
"except PermissionError:\n"
|
||||
" print('DENIED')"
|
||||
)
|
||||
assert result.success
|
||||
assert "DENIED" in result.stdout
|
||||
assert "WROTE" not in result.stdout
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_open_file_objects_are_not_iterable(self, temp_db_path):
|
||||
"""Pins the limitation the instructions warn about: pydantic/monty#490.
|
||||
|
||||
A failure here means Monty gained iteration support and the
|
||||
`for line in f` prohibition in the analysis instructions is now wrong.
|
||||
"""
|
||||
@pytest.mark.parametrize(
|
||||
"filename", ["content.txt", "items.jsonl", "toc.json", "metadata.json"]
|
||||
)
|
||||
async def test_write_denied_for_every_document_file(self, temp_db_path, filename):
|
||||
"""Every file in the document VFS is read-only, metadata.json included."""
|
||||
from docling_core.types.doc.document import DoclingDocument
|
||||
from docling_core.types.doc.labels import DocItemLabel
|
||||
|
||||
config = AppConfig()
|
||||
docling = DoclingDocument(name="d")
|
||||
docling.add_text(label=DocItemLabel.TEXT, text="one\ntwo")
|
||||
docling.add_text(label=DocItemLabel.TEXT, text="Foxes and dogs.")
|
||||
|
||||
async with HaikuRAG(temp_db_path, create=True) as client:
|
||||
doc = await client.import_document(
|
||||
docling,
|
||||
[
|
||||
Chunk(
|
||||
content="one\ntwo",
|
||||
content="Foxes and dogs.",
|
||||
embedding=[0.1] * config.embeddings.model.vector_dim,
|
||||
order=0,
|
||||
)
|
||||
],
|
||||
uri="test://lines",
|
||||
uri="test://readonly",
|
||||
)
|
||||
|
||||
sb = Sandbox(db_path=temp_db_path, config=config, context=AnalysisContext())
|
||||
try:
|
||||
result = await sb.execute(
|
||||
f"for line in open('/documents/{doc.id}/content.txt'):\n print(line)"
|
||||
)
|
||||
assert result.success is False
|
||||
assert "not iterable" in result.stderr
|
||||
|
||||
# The documented alternatives do work.
|
||||
result = await sb.execute(
|
||||
f"print(len(open('/documents/{doc.id}/content.txt').readlines()))"
|
||||
"from pathlib import Path\n"
|
||||
"try:\n"
|
||||
f" Path('/documents/{doc.id}/{filename}').write_text('nope')\n"
|
||||
" print('WROTE')\n"
|
||||
"except PermissionError:\n"
|
||||
" print('DENIED')"
|
||||
)
|
||||
assert result.success, result.stderr
|
||||
assert "DENIED" in result.stdout
|
||||
assert "WROTE" not in result.stdout
|
||||
finally:
|
||||
await sb.close()
|
||||
|
||||
|
|
@ -807,3 +780,42 @@ class TestSandboxReadDeadline:
|
|||
assert "no further document reads" in result.stderr
|
||||
finally:
|
||||
await sb.close()
|
||||
|
||||
|
||||
class TestSandboxWorkerCrash:
|
||||
"""A dead worker must not poison every later call in the run."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_crashed_worker_is_replaced(self, temp_db_path):
|
||||
"""The crash fails one call. The next call gets a fresh session."""
|
||||
import os
|
||||
import signal
|
||||
|
||||
config = AppConfig()
|
||||
async with HaikuRAG(temp_db_path, create=True):
|
||||
pass
|
||||
|
||||
sb = Sandbox(db_path=temp_db_path, config=config, context=AnalysisContext())
|
||||
try:
|
||||
first = await sb.execute("x = 1\nprint(x)")
|
||||
assert first.success, first.stderr
|
||||
assert sb._session is not None
|
||||
pid = sb._session.worker_pid
|
||||
assert pid is not None
|
||||
|
||||
os.kill(pid, signal.SIGKILL)
|
||||
|
||||
crashed = await sb.execute("print(2)")
|
||||
assert crashed.success is False
|
||||
assert "restarted" in crashed.stderr
|
||||
|
||||
# Without the discard this raises RuntimeError out of execute().
|
||||
recovered = await sb.execute("print(3)")
|
||||
assert recovered.success, recovered.stderr
|
||||
assert "3" in recovered.stdout
|
||||
|
||||
# The replacement worker starts empty, which the failure said.
|
||||
lost = await sb.execute("print(x)")
|
||||
assert lost.success is False
|
||||
finally:
|
||||
await sb.close()
|
||||
|
|
|
|||
|
|
@ -83,6 +83,23 @@ def _flatten(tree: list[dict]) -> list[dict]:
|
|||
return out
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestMetadataJson:
|
||||
"""metadata.json is served by a reader callback, like the other VFS files."""
|
||||
|
||||
async def test_metadata_reader_returns_document_fields(self, temp_db_path):
|
||||
async with HaikuRAG(temp_db_path, create=True) as client:
|
||||
doc_id = await _empty_doc(client, uri="test://meta", title="Meta Doc")
|
||||
|
||||
sandbox = Sandbox(temp_db_path, AppConfig(), AnalysisContext())
|
||||
raw = await _read_vfs_text(sandbox, f"/documents/{doc_id}/metadata.json")
|
||||
meta = json.loads(raw)
|
||||
|
||||
assert meta["id"] == doc_id
|
||||
assert meta["title"] == "Meta Doc"
|
||||
assert meta["uri"] == "test://meta"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
class TestTocShape:
|
||||
"""toc.json builds a section tree from heading_level + position."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue