fix(alerting): dedupe scheduled Slack spend reports across pods - #36489
Conversation
Every pod ran its own weekly/monthly spend report jobs, prometheus fallback stats cron, and daily report loop, so deployments with multiple replicas or uvicorn workers received one copy per pod. Gate each scheduled send behind the shared PodLockManager redis lock. The lock is never released: its TTL (the full reporting window for the weekly interval job, whose per-pod anchors drift by boot time and jitter) doubles as a sent-this-window marker. acquire_lock returning None (no redis wired) proceeds, preserving single-pod behavior. Also generalize the pod lock could-not-acquire log line, which claimed to be about spend tracking for every consumer. Fixes #14809
Weekly lock TTL gets an hour haircut: with ttl equal to the interval, the winner re-fires just before its own key expires, reacquires without a TTL refresh, and the key then lapses in time for a trailing pod to re-send. Job/lock ids move to litellm/constants.py per convention, and spend_report_frequency now rejects non-positive day counts, which previously coerced to an every-second schedule and would now compute a negative lock TTL that silently never sends. Adds the missing test coverage the review flagged: startup_event's pod_lock_manager wiring (identity-asserted), the prometheus closure's positive path, and the ungated immediate prometheus send pinned to exactly one await.
Drops a duplicate non-positive-days test and parametrizes the survivor over the suffix half of the validator too
Greptile SummaryThe PR coordinates scheduled Slack spend reports across replicas using shared Redis locks
Confidence Score: 5/5The PR appears safe to merge No blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Scheduled report wrappers consistently acquire non-reentrant shared locks, including both Prometheus startup and cron paths |
| litellm/proxy/db/db_transaction_queue/pod_lock_manager.py | Adds explicit non-reentrant semantics while preserving existing leader-election behavior by default |
| litellm/integrations/SlackAlerting/slack_alerting.py | Routes eligible daily reports through the shared window lock before sending |
| tests/test_litellm/proxy/proxy_server/test_lifecycle.py | Covers lock-held, lock-free, and unavailable-lock behavior for scheduled report jobs |
| tests/test_litellm/proxy/db/db_transaction_queue/test_pod_lock_manager.py | Verifies that non-reentrant acquisition rejects a live lock held by the same pod |
Reviews (3): Last reviewed commit: "fix(alerting): make report lock acquisit..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
… pod lock Greptile caught that the boot-time send still ran once per pod when PROMETHEUS_URL is set, the same duplication class this PR removes
|
@greptileai re-review please: the startup prometheus fallback send now goes through the same pod lock, tests updated accordingly |
Greptile caught that a pod booting within an hour of the fallback stats cron sent twice: the startup send takes the lock, then the cron fire hits acquire_lock's reacquire branch, which returns True for the holder. Window-marker gates now pass allow_reentrant=False so a live lock blocks everyone including its holder; leader-election consumers keep the reentrant default
|
@greptileai re-review: report gates now pass allow_reentrant=False so a live lock also blocks its own holder within the window |
_initialize_slack_alerting_jobs now reads it for the pod lock manager, and spec=ProxyLogging blocks instance-only attributes
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1884cc4. Configure here.
TLDR
Problem this solves:
How it solves it:
User Flow
Before: an ML platform team running the proxy with several replicas gets every Slack spend report several times
alerting: ["slack"],spend_report_frequency: "1d"and deploys 3 replicasAfter: the same config produces exactly one report per window
litellm_settings.cache,coordination_redis, oruse_redis_transaction_buffer) and deploys the same 3 replicasRelevant issues
Fixes #14809
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Two proxy instances simulating two pods (before run on ports 4113/4114, after run on 4115/4116), sharing one Postgres and one redis container that stayed up for every run; the runs differ only in whether litellm is wired to that redis. Config:
alerting: ["slack"],spend_report_frequency: "1d", plus alitellm_settings.cacheredis block in the wired variant only. Spend was seeded with a real groq chat completion through instance A; the weekly job fires 10-310s after each instance bootsBefore, at 363d56f, redis running but not wired into litellm: each pod sends its own copy of the same report
After, at 3f0a793, redis wired via
litellm_settings.cache: exactly one report, and a 2s loop deleted the pre-existingweekly_spend_report_sent_*cache key throughout the run, so the old date-window dedupe could not be what suppressed the second sendThe review-found TTL boundary fix is confirmed by reading the lock back right after acquisition:
redis-cli ttl cronjob_lock:weekly_spend_report_jobreturned 82794 (the 1d window minus the hour margin, minus 7s elapsed); the same read against the pre-fix TTL gave 86391, i.e. a TTL equal to the job interval, which is the boundary defect described aboveSame fixed commit with redis unwired still duplicates (both pods proceed on the no-redis path), which is the first caveat below
Commits after 3f0a793 address Greptile findings: the startup prometheus fallback send now routes through the same gated closure, and gates pass allow_reentrant=False so a pod booting within an hour of the fallback cron cannot send twice; both are pinned by unit tests asserting the exact lock calls
Type
🐛 Bug Fix
Caveats (if any)
spend_report_frequency: "0d"now fails startup; it used to coerce to an every-second scheduleFinal Attestation
Note
Cursor Bugbot is generating a summary for commit 1884cc4. Configure here.