-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix(proxy): prevent OOM/Prisma connection loss from unbounded managed-object poll #23472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1f3d128
b2252b4
5acd8f6
e7f8cd5
52c0574
a14b951
679b3b2
b75f616
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,11 +2,15 @@ | |||||||||||||||||||||||||||||||||||||||||
| Polls LiteLLM_ManagedObjectTable to check if the batch job is complete, and if the cost has been tracked. | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| from litellm._uuid import uuid | ||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime, timedelta, timezone | ||||||||||||||||||||||||||||||||||||||||||
| from typing import TYPE_CHECKING, Optional | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| from litellm._logging import verbose_proxy_logger | ||||||||||||||||||||||||||||||||||||||||||
| from litellm._uuid import uuid | ||||||||||||||||||||||||||||||||||||||||||
| from litellm.constants import ( | ||||||||||||||||||||||||||||||||||||||||||
| MANAGED_OBJECT_STALENESS_CUTOFF_DAYS, | ||||||||||||||||||||||||||||||||||||||||||
| MAX_OBJECTS_PER_POLL_CYCLE, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||||||||||||||||||||||||||
| from litellm.proxy.utils import PrismaClient, ProxyLogging | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -29,6 +33,9 @@ def __init__( | |||||||||||||||||||||||||||||||||||||||||
| self.proxy_logging_obj: ProxyLogging = proxy_logging_obj | ||||||||||||||||||||||||||||||||||||||||||
| self.prisma_client: PrismaClient = prisma_client | ||||||||||||||||||||||||||||||||||||||||||
| self.llm_router: Router = llm_router | ||||||||||||||||||||||||||||||||||||||||||
| # Cached after the first poll cycle. Once we know the column is absent we skip | ||||||||||||||||||||||||||||||||||||||||||
| # the guaranteed-failing primary query on every subsequent cycle. | ||||||||||||||||||||||||||||||||||||||||||
| self._has_batch_processed_column: bool = True | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| async def _get_user_info(self, batch_id, user_id) -> dict: | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -49,6 +56,47 @@ async def _get_user_info(self, batch_id, user_id) -> dict: | |||||||||||||||||||||||||||||||||||||||||
| verbose_proxy_logger.error(f"CheckBatchCost: could not look up user {user_id} for batch {batch_id}: {e}") | ||||||||||||||||||||||||||||||||||||||||||
| return {} | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| async def _cleanup_stale_managed_objects(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
| Mark managed objects older than MANAGED_OBJECT_STALENESS_CUTOFF_DAYS days | ||||||||||||||||||||||||||||||||||||||||||
| in non-terminal states as 'stale_expired'. These will never complete and | ||||||||||||||||||||||||||||||||||||||||||
| should not be polled. | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
| cutoff = datetime.now(timezone.utc) - timedelta(days=MANAGED_OBJECT_STALENESS_CUTOFF_DAYS) | ||||||||||||||||||||||||||||||||||||||||||
| result = await self.prisma_client.db.litellm_managedobjecttable.update_many( | ||||||||||||||||||||||||||||||||||||||||||
| where={ | ||||||||||||||||||||||||||||||||||||||||||
| "file_purpose": "batch", | ||||||||||||||||||||||||||||||||||||||||||
| "status": {"not_in": ["completed", "complete", "failed", "expired", "cancelled", "stale_expired"]}, | ||||||||||||||||||||||||||||||||||||||||||
| "created_at": {"lt": cutoff}, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| data={"status": "stale_expired"}, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| if result > 0: | ||||||||||||||||||||||||||||||||||||||||||
| verbose_proxy_logger.warning( | ||||||||||||||||||||||||||||||||||||||||||
| f"CheckBatchCost: marked {result} stale managed objects " | ||||||||||||||||||||||||||||||||||||||||||
| f"(older than {MANAGED_OBJECT_STALENESS_CUTOFF_DAYS} days) as stale_expired" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| async def _fallback_find_jobs(self) -> list: | ||||||||||||||||||||||||||||||||||||||||||
| """Query batch jobs without the batch_processed filter (for older schemas).""" | ||||||||||||||||||||||||||||||||||||||||||
| return await self.prisma_client.db.litellm_managedobjecttable.find_many( | ||||||||||||||||||||||||||||||||||||||||||
| where={ | ||||||||||||||||||||||||||||||||||||||||||
| "file_purpose": "batch", | ||||||||||||||||||||||||||||||||||||||||||
| "status": { | ||||||||||||||||||||||||||||||||||||||||||
| "not_in": [ | ||||||||||||||||||||||||||||||||||||||||||
| "failed", | ||||||||||||||||||||||||||||||||||||||||||
| "expired", | ||||||||||||||||||||||||||||||||||||||||||
| "cancelled", | ||||||||||||||||||||||||||||||||||||||||||
| "complete", | ||||||||||||||||||||||||||||||||||||||||||
| "completed", | ||||||||||||||||||||||||||||||||||||||||||
| "stale_expired", | ||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| take=MAX_OBJECTS_PER_POLL_CYCLE, | ||||||||||||||||||||||||||||||||||||||||||
| order={"created_at": "asc"}, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| async def check_batch_cost(self): | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
| Check if the batch JOB has been tracked. | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -70,14 +118,48 @@ async def check_batch_cost(self): | |||||||||||||||||||||||||||||||||||||||||
| get_model_id_from_unified_batch_id, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # Look for all batches that have not yet been processed by CheckBatchCost | ||||||||||||||||||||||||||||||||||||||||||
| jobs = await self.prisma_client.db.litellm_managedobjecttable.find_many( | ||||||||||||||||||||||||||||||||||||||||||
| where={ | ||||||||||||||||||||||||||||||||||||||||||
| "file_purpose": "batch", | ||||||||||||||||||||||||||||||||||||||||||
| "batch_processed" : False, | ||||||||||||||||||||||||||||||||||||||||||
| "status": {"not_in": ["failed", "expired", "cancelled"]} | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||
| await self._cleanup_stale_managed_objects() | ||||||||||||||||||||||||||||||||||||||||||
| except Exception as cleanup_err: | ||||||||||||||||||||||||||||||||||||||||||
| verbose_proxy_logger.warning( | ||||||||||||||||||||||||||||||||||||||||||
| f"CheckBatchCost: stale cleanup failed (poll will continue): {cleanup_err}" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # Look for all batches that have not yet been processed by CheckBatchCost. | ||||||||||||||||||||||||||||||||||||||||||
| # self._has_batch_processed_column is cached after the first probe so that | ||||||||||||||||||||||||||||||||||||||||||
| # older schemas don't pay a guaranteed-failing primary query + warning on | ||||||||||||||||||||||||||||||||||||||||||
| # every subsequent poll cycle. | ||||||||||||||||||||||||||||||||||||||||||
| if self._has_batch_processed_column: | ||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||
| jobs = await self.prisma_client.db.litellm_managedobjecttable.find_many( | ||||||||||||||||||||||||||||||||||||||||||
| where={ | ||||||||||||||||||||||||||||||||||||||||||
| "file_purpose": "batch", | ||||||||||||||||||||||||||||||||||||||||||
| "batch_processed": False, | ||||||||||||||||||||||||||||||||||||||||||
| "status": { | ||||||||||||||||||||||||||||||||||||||||||
| "not_in": [ | ||||||||||||||||||||||||||||||||||||||||||
| "failed", | ||||||||||||||||||||||||||||||||||||||||||
| "expired", | ||||||||||||||||||||||||||||||||||||||||||
| "cancelled", | ||||||||||||||||||||||||||||||||||||||||||
| "complete", | ||||||||||||||||||||||||||||||||||||||||||
| "completed", | ||||||||||||||||||||||||||||||||||||||||||
| "stale_expired", | ||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| take=MAX_OBJECTS_PER_POLL_CYCLE, | ||||||||||||||||||||||||||||||||||||||||||
| order={"created_at": "asc"}, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+132
to
+151
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Primary query's The fallback query correctly excludes The fallback avoids this by explicitly listing terminal statuses in its
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| except Exception as query_err: | ||||||||||||||||||||||||||||||||||||||||||
| if "batch_processed" not in str(query_err).lower() and "unknown column" not in str(query_err).lower() and "does not exist" not in str(query_err).lower(): | ||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+152
to
+154
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Broad The three-string filter correctly handles
When any of these strings matches, The specific "relation does not exist" case is the most dangerous: the table being absent would cause the fallback to also fail, so the wrong There is no test covering the "non-schema error should be re-raised without touching the cache" path. Consider narrowing the pattern to be Prisma/column-specific: except Exception as query_err:
err_str = str(query_err).lower()
is_schema_error = (
("batch_processed" in err_str and "does not exist" in err_str)
or ("column" in err_str and "does not exist" in err_str)
or "unknown column" in err_str
)
if not is_schema_error:
raise
self._has_batch_processed_column = False
...This restricts the match to errors that explicitly mention a column not existing, preventing relation-level or transient errors from triggering the cache update. |
||||||||||||||||||||||||||||||||||||||||||
| # Permanent schema gap β cache the result so future cycles skip straight to fallback | ||||||||||||||||||||||||||||||||||||||||||
| self._has_batch_processed_column = False | ||||||||||||||||||||||||||||||||||||||||||
| verbose_proxy_logger.warning( | ||||||||||||||||||||||||||||||||||||||||||
| "CheckBatchCost: batch_processed column not found, querying without it" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| jobs = await self._fallback_find_jobs() | ||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||
| jobs = await self._fallback_find_jobs() | ||||||||||||||||||||||||||||||||||||||||||
| for job in jobs: | ||||||||||||||||||||||||||||||||||||||||||
| # get the model from the job | ||||||||||||||||||||||||||||||||||||||||||
| unified_object_id = job.unified_object_id | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -163,14 +245,14 @@ async def check_batch_cost(self): | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # Access content - handle both direct attribute and method call | ||||||||||||||||||||||||||||||||||||||||||
| if hasattr(_file_content, 'content'): | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes = _file_content.content | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes = _file_content.content # type: ignore[union-attr] | ||||||||||||||||||||||||||||||||||||||||||
| elif hasattr(_file_content, 'read'): | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes = await _file_content.read() | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes = await _file_content.read() # type: ignore[misc] | ||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes = _file_content | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes = _file_content # type: ignore[assignment] | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| file_content_as_dict = _get_file_content_as_dictionary( | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes | ||||||||||||||||||||||||||||||||||||||||||
| content_bytes # type: ignore[arg-type] | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| deployment_info = self.llm_router.get_deployment(model_id=model_id) | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -195,7 +277,7 @@ async def check_batch_cost(self): | |||||||||||||||||||||||||||||||||||||||||
| file_content_dictionary=file_content_as_dict, | ||||||||||||||||||||||||||||||||||||||||||
| custom_llm_provider=llm_provider, # type: ignore | ||||||||||||||||||||||||||||||||||||||||||
| model_name=model_name, | ||||||||||||||||||||||||||||||||||||||||||
| model_info=deployment_model_info, | ||||||||||||||||||||||||||||||||||||||||||
| model_info=deployment_model_info, # type: ignore[arg-type] | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| logging_obj = LiteLLMLogging( | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -236,13 +318,15 @@ async def check_batch_cost(self): | |||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # mark the job as complete | ||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||
| update_data: dict = { | ||||||||||||||||||||||||||||||||||||||||||
| "status": "complete", | ||||||||||||||||||||||||||||||||||||||||||
| "file_object": response.model_dump_json(), | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| if self._has_batch_processed_column: | ||||||||||||||||||||||||||||||||||||||||||
| update_data["batch_processed"] = True | ||||||||||||||||||||||||||||||||||||||||||
| await self.prisma_client.db.litellm_managedobjecttable.update( | ||||||||||||||||||||||||||||||||||||||||||
| where={"id": job.id}, | ||||||||||||||||||||||||||||||||||||||||||
| data={ | ||||||||||||||||||||||||||||||||||||||||||
| "batch_processed": True, | ||||||||||||||||||||||||||||||||||||||||||
| "status": "complete", | ||||||||||||||||||||||||||||||||||||||||||
| "file_object": response.model_dump_json(), | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| data=update_data, | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| except Exception as db_err: | ||||||||||||||||||||||||||||||||||||||||||
| verbose_proxy_logger.error( | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -214,6 +214,7 @@ def generate_feedback_box(): | |
| DEFAULT_MODEL_CREATED_AT_TIME, | ||
| LITELLM_PROXY_ADMIN_NAME, | ||
| PROMETHEUS_FALLBACK_STATS_SEND_TIME_HOURS, | ||
| PROXY_BATCH_POLLING_ENABLED, | ||
| PROXY_BATCH_POLLING_INTERVAL, | ||
| PROXY_BATCH_WRITE_AT, | ||
| PROXY_BUDGET_RESCHEDULER_MAX_TIME, | ||
|
|
@@ -260,7 +261,6 @@ def generate_feedback_box(): | |
| claude_code_marketplace_router, | ||
| ) | ||
| from litellm.proxy.anthropic_endpoints.endpoints import router as anthropic_router | ||
| from litellm.proxy.realtime_endpoints.endpoints import router as webrtc_router | ||
| from litellm.proxy.anthropic_endpoints.skills_endpoints import ( | ||
| router as anthropic_skills_router, | ||
| ) | ||
|
|
@@ -471,6 +471,7 @@ def generate_feedback_box(): | |
| from litellm.proxy.prompts.prompt_endpoints import router as prompts_router | ||
| from litellm.proxy.public_endpoints import router as public_endpoints_router | ||
| from litellm.proxy.rag_endpoints.endpoints import router as rag_router | ||
| from litellm.proxy.realtime_endpoints.endpoints import router as webrtc_router | ||
| from litellm.proxy.rerank_endpoints.endpoints import router as rerank_router | ||
| from litellm.proxy.response_api_endpoints.endpoints import router as response_router | ||
| from litellm.proxy.route_llm_request import route_request | ||
|
|
@@ -6069,7 +6070,7 @@ async def initialize_scheduled_background_jobs( # noqa: PLR0915 | |
| "Invalid maximum_spend_logs_retention_interval value" | ||
| ) | ||
| ### CHECK BATCH COST ### | ||
| if llm_router is not None: | ||
| if llm_router is not None and PROXY_BATCH_POLLING_ENABLED: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The kill-switch disables both Similarly, a user reading Consider either:
The documentation entry in |
||
| try: | ||
| from litellm_enterprise.proxy.common_utils.check_batch_cost import ( | ||
| CheckBatchCost, | ||
|
|
@@ -6100,7 +6101,7 @@ async def initialize_scheduled_background_jobs( # noqa: PLR0915 | |
| pass | ||
|
|
||
| ### CHECK RESPONSES COST ### | ||
| if llm_router is not None: | ||
| if llm_router is not None and PROXY_BATCH_POLLING_ENABLED: | ||
| try: | ||
| from litellm_enterprise.proxy.common_utils.check_responses_cost import ( | ||
| CheckResponsesCost, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate stale cleanup runs twice per poll cycle
CheckBatchCost._cleanup_stale_managed_objects()andCheckResponsesCost._cleanup_stale_managed_objects()are identical β both issue the exact sameupdate_manyagainst the fullLiteLLM_ManagedObjectTablewith nofile_purposefilter. Since both jobs are scheduled at the same interval inproxy_server.py, every poll cycle triggers two separate full-table stale-row scans on the same set of rows.The second run is a no-op (rows already marked
stale_expiredare excluded by thenot_infilter), but it still incurs a full DB round-trip that re-scans the same large table you were trying to avoid loading into memory. At 336 K rows this doubles the remediation cost of the very problem being fixed.Adding a
file_purposefilter to each class's cleanup scopes the update to its own object type and eliminates the redundant scan: