fix(review): require graphics_limit>=1, type _index_document, cover rlimit branch
Address PR #852 round 2: - config: DOCUMENT_PDF_GRAPHICS_LIMIT validator is now gte=1 (pymupdf4llm treats 0 as "no cap", which would re-expose the OOM); documented the zero semantics and that the per-worker mem rlimit needs a pod restart to change. - processor: annotate `_index_document -> bool | None` and document the contract so the `if indexed is False` check is explicit/type-checkable. - tests: add the RLIM_INFINITY-hard branch assertion for _apply_mem_limit (soft==target, hard stays unbounded). 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
7ec116a3c7
commit
6589e8e7fc
@@ -281,9 +281,11 @@ _dynaconf = Dynaconf(
|
|||||||
Validator("DOCUMENT_CHUNK_SIZE", gte=1),
|
Validator("DOCUMENT_CHUNK_SIZE", gte=1),
|
||||||
Validator("DOCUMENT_PARSE_TIMEOUT_SECONDS", gte=1),
|
Validator("DOCUMENT_PARSE_TIMEOUT_SECONDS", gte=1),
|
||||||
Validator("DOCUMENT_PARSE_MEM_LIMIT_MB", gte=128),
|
Validator("DOCUMENT_PARSE_MEM_LIMIT_MB", gte=128),
|
||||||
|
# >=1: pymupdf4llm treats graphics_limit=0 as "no cap", which would
|
||||||
|
# re-expose the OOM this guards against.
|
||||||
|
Validator("DOCUMENT_PDF_GRAPHICS_LIMIT", gte=1),
|
||||||
# Non-negative
|
# Non-negative
|
||||||
Validator("DOCUMENT_CHUNK_OVERLAP", gte=0),
|
Validator("DOCUMENT_CHUNK_OVERLAP", gte=0),
|
||||||
Validator("DOCUMENT_PDF_GRAPHICS_LIMIT", gte=0),
|
|
||||||
# Non-empty strings
|
# Non-empty strings
|
||||||
Validator("VECTOR_SYNC_PDF_TAG", len_min=1),
|
Validator("VECTOR_SYNC_PDF_TAG", len_min=1),
|
||||||
# Enum constraints
|
# Enum constraints
|
||||||
@@ -710,15 +712,14 @@ class Settings:
|
|||||||
|
|
||||||
# PDF parse isolation (OOM guard). The parse runs in a subprocess so one
|
# PDF parse isolation (OOM guard). The parse runs in a subprocess so one
|
||||||
# pathological file fails that doc, not the pod.
|
# pathological file fails that doc, not the pod.
|
||||||
document_pdf_graphics_limit: int = (
|
# to_markdown graphics cap; pages above it skip graphics analysis. Must be
|
||||||
5000 # to_markdown graphics cap; pages above skip graphics analysis
|
# >=1 -- pymupdf4llm treats 0 as "no cap", which re-exposes the OOM.
|
||||||
)
|
document_pdf_graphics_limit: int = 5000
|
||||||
document_parse_timeout_seconds: int = (
|
# wall-clock cap per parse; the worker subprocess is killed on timeout.
|
||||||
120 # wall-clock cap per parse; the worker subprocess is killed on timeout
|
document_parse_timeout_seconds: int = 120
|
||||||
)
|
# RLIMIT_AS in the parse subprocess (below the pod limit). Applied once per
|
||||||
document_parse_mem_limit_mb: int = (
|
# worker for its lifetime, so changing it needs a pod restart.
|
||||||
1536 # RLIMIT_AS in the parse subprocess (kept below the pod memory limit)
|
document_parse_mem_limit_mb: int = 1536
|
||||||
)
|
|
||||||
|
|
||||||
# Observability settings
|
# Observability settings
|
||||||
metrics_enabled: bool = True
|
metrics_enabled: bool = True
|
||||||
|
|||||||
@@ -311,10 +311,13 @@ async def process_document(
|
|||||||
|
|
||||||
async def _index_document(
|
async def _index_document(
|
||||||
doc_task: DocumentTask, nc_client: NextcloudClient, qdrant_client
|
doc_task: DocumentTask, nc_client: NextcloudClient, qdrant_client
|
||||||
):
|
) -> bool | None:
|
||||||
"""
|
"""
|
||||||
Index a single document (called by process_document with retry).
|
Index a single document (called by process_document with retry).
|
||||||
|
|
||||||
|
Returns ``False`` when a permanent parse failure means nothing was indexed
|
||||||
|
(the caller must then skip the success metrics); ``None`` otherwise.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
doc_task: Document task to index
|
doc_task: Document task to index
|
||||||
nc_client: Authenticated Nextcloud client
|
nc_client: Authenticated Nextcloud client
|
||||||
|
|||||||
@@ -129,6 +129,24 @@ def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch):
|
|||||||
assert hard == 4 * 1024**3
|
assert hard == 4 * 1024**3
|
||||||
|
|
||||||
|
|
||||||
|
def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch):
|
||||||
|
captured = {}
|
||||||
|
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
_isolation.resource,
|
||||||
|
"getrlimit",
|
||||||
|
lambda _w: (resource.RLIM_INFINITY, resource.RLIM_INFINITY),
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
_isolation.resource, "setrlimit", lambda _w, pair: captured.update(pair=pair)
|
||||||
|
)
|
||||||
|
# hard is unbounded -> soft is exactly the target, hard stays RLIM_INFINITY
|
||||||
|
_isolation._apply_mem_limit(1536)
|
||||||
|
soft, hard = captured["pair"]
|
||||||
|
assert soft == 1536 * 1024 * 1024
|
||||||
|
assert hard == resource.RLIM_INFINITY
|
||||||
|
|
||||||
|
|
||||||
def test_apply_mem_limit_is_applied_once(monkeypatch):
|
def test_apply_mem_limit_is_applied_once(monkeypatch):
|
||||||
calls = []
|
calls = []
|
||||||
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
|
||||||
|
|||||||
Reference in New Issue
Block a user