fix(review): type timeout as float; document worker reuse + identity check
Address PR #852 round 3 (all 🟡, no blockers): - config: DOCUMENT_PARSE_TIMEOUT_SECONDS is now float (default 120.0) so a fractional value is honoured rather than silently stored in an int field; matches anyio.move_on_after's float seconds. - _isolation: comment that a clean rlimit MemoryError leaves the worker alive in anyio's pool (vs the SIGKILL/BrokenWorkerProcess path that respawns) -- acceptable since RLIMIT_AS caps virtual address space, not RSS. - processor: note the `if indexed is False` is a deliberate identity check -- a successful index (incl. dedup hit) returns None and must not be mistaken for a parse failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
6589e8e7fc
commit
93f0f4f881
@@ -134,7 +134,7 @@ _DEFAULTS: dict[str, Any] = {
|
||||
"document_chunk_overlap": 200,
|
||||
# PDF parse isolation (OOM guard)
|
||||
"document_pdf_graphics_limit": 5000,
|
||||
"document_parse_timeout_seconds": 120,
|
||||
"document_parse_timeout_seconds": 120.0,
|
||||
"document_parse_mem_limit_mb": 1536,
|
||||
# Observability
|
||||
"metrics_enabled": True,
|
||||
@@ -716,7 +716,9 @@ class Settings:
|
||||
# >=1 -- pymupdf4llm treats 0 as "no cap", which re-exposes the OOM.
|
||||
document_pdf_graphics_limit: int = 5000
|
||||
# wall-clock cap per parse; the worker subprocess is killed on timeout.
|
||||
document_parse_timeout_seconds: int = 120
|
||||
# float so a fractional DOCUMENT_PARSE_TIMEOUT_SECONDS is honoured, matching
|
||||
# anyio.move_on_after's float seconds.
|
||||
document_parse_timeout_seconds: float = 120.0
|
||||
# RLIMIT_AS in the parse subprocess (below the pod limit). Applied once per
|
||||
# worker for its lifetime, so changing it needs a pod restart.
|
||||
document_parse_mem_limit_mb: int = 1536
|
||||
|
||||
@@ -115,6 +115,11 @@ async def run_isolated_pdf_parse(
|
||||
cancellable=True,
|
||||
)
|
||||
except MemoryError as e:
|
||||
# A clean rlimit breach: the worker raised MemoryError and stays
|
||||
# ALIVE in anyio's pool (unlike the BrokenWorkerProcess/SIGKILL path,
|
||||
# which spawns a fresh worker). Its heap may be slightly fragmented
|
||||
# for the next document. Acceptable: RLIMIT_AS caps virtual address
|
||||
# space (not RSS), so practical fragmentation risk is low.
|
||||
raise PdfParseFailed("oom", str(e)) from e
|
||||
except BrokenWorkerProcess as e:
|
||||
# Worker died without a clean exception (e.g. SIGKILL from the OS OOM
|
||||
|
||||
@@ -244,6 +244,9 @@ async def process_document(
|
||||
# document_parse_total{error}) and the placeholder marked
|
||||
# "failed". It is not an indexing event and not retryable, so
|
||||
# don't count it as a successful upsert/indexed document.
|
||||
# Identity check, not `if not indexed`: a successful index
|
||||
# (including a dedup hit) returns None, which must NOT be
|
||||
# treated as a parse failure.
|
||||
if indexed is False:
|
||||
return
|
||||
|
||||
|
||||
Reference in New Issue
Block a user