Marker retries - #304
Conversation
Extract retry_with_backoff() helper in components/ray_utils.py and wire it into MarkerPool, DoclingPool, and WhisperPool so transient worker failures (GPU OOM, broken ProcessPoolExecutor, etc.) retry per-task instead of failing the whole document. Retries happen at the pool layer so each attempt re-acquires a worker from the queue and re-runs health checks. Marker keeps per-chunk granularity — a failed chunk retries without re-running sibling chunks or post-processing. Also adds timeout coverage to DoclingPool and WhisperPool, which previously awaited actor.remote() directly with no timeout or ray.cancel on caller cancellation. Per-loader knobs (marker/docling default 3 retries; whisper default 1 since failures there are often deterministic like corrupt audio).
b631fea to
903f8f3
Compare
|
@coderabbitai review |
|
@coderabbitai review now you're awake |
|
✅ Actions performedReview triggered.
|
|
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR adds configurable timeout and exponential backoff retry mechanisms to three Ray-based file processing components: local whisper (audio transcription), marker (PDF chunking), and docling (PDF conversion). A new Changes
Sequence DiagramsequenceDiagram
participant Loader Component
participant retry_with_backoff
participant call_ray_actor_with_timeout
participant Ray Actor
Loader Component->>retry_with_backoff: attempt_fn, max_retries, base_delay
loop For each attempt (0 to max_retries)
retry_with_backoff->>Loader Component: attempt(attempt_num)
Loader Component->>Loader Component: acquire resource from queue
Loader Component->>call_ray_actor_with_timeout: invoke with timeout
alt Task succeeds within timeout
call_ray_actor_with_timeout->>Ray Actor: remote call
Ray Actor-->>call_ray_actor_with_timeout: result
call_ray_actor_with_timeout-->>Loader Component: return result
Loader Component->>Loader Component: return resource to queue
Loader Component-->>retry_with_backoff: success
retry_with_backoff-->>Loader Component: return result
else Timeout or Exception
call_ray_actor_with_timeout-->>Loader Component: raise exception
Loader Component->>Loader Component: return resource to queue
Loader Component-->>retry_with_backoff: raise exception
alt Final attempt
retry_with_backoff->>retry_with_backoff: log error, re-raise
retry_with_backoff-->>Loader Component: raise exception
else Intermediate attempt
retry_with_backoff->>retry_with_backoff: log warning, wait exponential delay
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
openrag/config/models.py (1)
344-351: Consider adding validation constraints for consistency.The nearby
docling_num_gpusanddocling_pool_sizefields useField(ge=...)constraints, but the new retry/timeout fields don't. For consistency and safety, consider adding validation to prevent invalid configurations (e.g., negative retries or delays).🔧 Optional: Add validation constraints
- marker_max_task_retry: int = 3 - marker_retry_base_delay: float = 2.0 + marker_max_task_retry: int = Field(default=3, ge=0) + marker_retry_base_delay: float = Field(default=2.0, ge=0) docling_num_gpus: float = Field(default=0.01, ge=0) docling_pool_size: int = Field(default=1, ge=1) docling_max_tasks_per_worker: int = Field(default=2, ge=1) - docling_timeout: int = 3600 - docling_max_task_retry: int = 3 - docling_retry_base_delay: float = 2.0 + docling_timeout: int = Field(default=3600, gt=0) + docling_max_task_retry: int = Field(default=3, ge=0) + docling_retry_base_delay: float = Field(default=2.0, ge=0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openrag/config/models.py` around lines 344 - 351, Add validation constraints to the new retry/timeout fields to prevent invalid values: change marker_max_task_retry and docling_max_task_retry to use Field(default=3, ge=0) to forbid negatives, set marker_retry_base_delay and docling_retry_base_delay to Field(default=2.0, ge=0.0) to forbid negative delays, and make docling_timeout use Field(default=3600, ge=0) (or ge=1 if zero timeout is invalid); update the Field() calls for these symbols (marker_max_task_retry, marker_retry_base_delay, docling_timeout, docling_max_task_retry, docling_retry_base_delay) accordingly to match the pattern used by docling_num_gpus and docling_pool_size.openrag/components/ray_utils.py (1)
61-66: Type hint should indicate async callable.The
attempt_fnparameter is always awaited (line 76), so the type hint should reflect that it returns an awaitable rather thanAny.🔧 Suggested type hint improvement
+from collections.abc import Awaitable, Callable -from collections.abc import Callable from typing import Any ... async def retry_with_backoff( - attempt_fn: Callable[[int], Any], + attempt_fn: Callable[[int], Awaitable[Any]], max_retries: int, base_delay: float, task_description: str = "task", ) -> Any:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openrag/components/ray_utils.py` around lines 61 - 66, The retry_with_backoff signature currently types attempt_fn as Callable[[int], Any] but attempt_fn is awaited inside the async function; update the signature to use a generic TypeVar (e.g., T) and annotate attempt_fn as Callable[[int], Awaitable[T]] and the function return type as -> T; add the necessary imports (TypeVar, Awaitable) and update any related hints to use the TypeVar so type checkers correctly infer the awaited result for retry_with_backoff.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@openrag/components/ray_utils.py`:
- Around line 61-66: The retry_with_backoff signature currently types attempt_fn
as Callable[[int], Any] but attempt_fn is awaited inside the async function;
update the signature to use a generic TypeVar (e.g., T) and annotate attempt_fn
as Callable[[int], Awaitable[T]] and the function return type as -> T; add the
necessary imports (TypeVar, Awaitable) and update any related hints to use the
TypeVar so type checkers correctly infer the awaited result for
retry_with_backoff.
In `@openrag/config/models.py`:
- Around line 344-351: Add validation constraints to the new retry/timeout
fields to prevent invalid values: change marker_max_task_retry and
docling_max_task_retry to use Field(default=3, ge=0) to forbid negatives, set
marker_retry_base_delay and docling_retry_base_delay to Field(default=2.0,
ge=0.0) to forbid negative delays, and make docling_timeout use
Field(default=3600, ge=0) (or ge=1 if zero timeout is invalid); update the
Field() calls for these symbols (marker_max_task_retry, marker_retry_base_delay,
docling_timeout, docling_max_task_retry, docling_retry_base_delay) accordingly
to match the pattern used by docling_num_gpus and docling_pool_size.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0abfa78-da04-42d0-a669-2de5f453069c
📒 Files selected for processing (6)
conf/config.yamlopenrag/components/indexer/loaders/audio/local_whisper.pyopenrag/components/indexer/loaders/pdf_loaders/docling2.pyopenrag/components/indexer/loaders/pdf_loaders/marker.pyopenrag/components/ray_utils.pyopenrag/config/models.py
Summary by CodeRabbit
Release Notes