Skip to content

test(e2e): skip the batch rate-limiter spend-row test pending LIT-5027 - #35301

Merged
ryan-crabbe-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_skip_batch_rl_unattributed_spend_e2e
Jul 31, 2026
Merged

test(e2e): skip the batch rate-limiter spend-row test pending LIT-5027#35301
ryan-crabbe-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_skip_batch_rl_unattributed_spend_e2e

Conversation

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor

TLDR

High level flow for the user:

  • Nothing changes for users; test-only
  • Batch e2e no longer reports a false timeout

High level flow on a technical level:

  • Batch rate limiter awaits file read with no timeout
  • Hang eats the test's 60s budget before its assertion
  • Skip the test, reason names LIT-5027

Relevant issues

Linear ticket

Refs LIT-5027

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

This PR adds no product behavior, so there is nothing to curl a green result out of; the thing being proven is that the path under test hangs server-side and that the test can never reach its assertion. Evidence is from the stage e2e run on 2026-07-30 12:42 UTC, gateway pod litellm-gateway-5645668d45-xthvk in namespace litellm, at stage image commit 38f2e023f1

The client issued POST /v1/batches at 12:50:05Z right after its input file uploaded cleanly, and gave up at its 60s read timeout:

12:50:05.189  INFO: 192.168.118.11:2392 - "POST /v1/files?model=openai-batch-5a59e9711817 HTTP/1.1" 200 OK
12:51:05.73   test_rate_limited_batch_create_leaves_no_unattributed_spend_row FAILED [  6%]

E  Failed: upstream call failed (status -1); body=HTTPConnectionPool(
   host='internal-k8s-litellm-litellm-e7f4afb143-1355079179.us-east-1.elb.amazonaws.com',
   port=80): Read timed out. (read timeout=60.0)

No access-log line is ever emitted for that POST /v1/batches, and both the gateway and backend pods go silent for the full 60s apart from health probes. The read finally resolves at 12:51:09.33Z, 63.6 seconds after it started, inside the rate limiter:

batch_rate_limiter.py:608  Error counting input file usage for file-...:
Error code: 404 - {'error': {'message': 'No such File object: file-CV64vPhJTUoLCw24TB5kmd', ...}}

batch_rate_limiter.py:858  Error in batch rate limiting: ...
  File "/app/litellm/proxy/hooks/batch_rate_limiter.py", line 827, in async_pre_call_hook
    batch_usage = await self.count_input_file_usage(
  File "/app/litellm/proxy/hooks/batch_rate_limiter.py", line 533, in count_input_file_usage
    file_content = await litellm.afile_content(
  File "/app/litellm/llms/openai/openai.py", line 1672, in afile_content
    response = await openai_client.files.content(**file_content_request)

The 404 is a consequence of the client's teardown deleting the file it was still reading, not the cause of the hang. Pod restart, OOM, SIGTERM and DB slowness are all ruled out: that gateway pod served continuously and did not log Shutting down until 14:05:15Z, 74 minutes later

The unbounded await is visible in the tree; grep -n timeout litellm/proxy/hooks/batch_rate_limiter.py returns no matches, so litellm.afile_content inherits the OpenAI SDK default of 600s with two retries. This is not a regression; the call has been unbounded since the hook landed in #16075 on 2025-10-29

After this PR, at commit 010bf6f793, the test is skipped and the reason travels with it:

$ cd tests/e2e && python -m pytest batches/test_batches_e2e.py -k rate_limited_batch_create -rs
s                                                                        [100%]
SKIPPED [1] batches/test_batches_e2e.py:400: LIT-5027: the path under test hangs. ...
1 skipped, 22 deselected in 0.03s

Type

✅ Test

Changes

Adds a pytest.mark.skip to test_rate_limited_batch_create_leaves_no_unattributed_spend_row with a reason naming LIT-5027 and describing why the path cannot be exercised today. No other test is touched and no product code changes

The test is the only coverage of the batch rate limiter's input-file read; an unlimited key skips that path entirely, which is why the test deliberately mints a key with generous rpm/tpm limits. Skipping it therefore leaves the LIT-3266 unattributed-spend-row contract uncovered, and that tradeoff is called out in LIT-5027 so the skip is lifted rather than forgotten. The alternative, raising the harness read timeout past the hang, was rejected: it would turn a product defect into a slow green test

QA runbook

  • tests/e2e/batches/test_batches_e2e.py::test_rate_limited_batch_create_leaves_no_unattributed_spend_row - currently skipped; when unskipped it proves that creating a batch on a rate-limited key adds no spend row that lacks a caller identity
    • Mints a key directly on the gateway with tpm_limit=1_000_000 and rpm_limit=1_000 rather than through resources.key(), because the limiter only reads the input file when the key carries applicable rpm/tpm limits, and the generous values keep the batch itself unblocked
    • Snapshots the request ids of every unattributed spend row (rows with an empty api_key) over a bounded two-hour window via /spend/logs/v2, so the later comparison measures only what this test introduced and does not read the whole table
    • Uploads a batch JSONL input file under the batch purpose and defers its deletion, giving the limiter something to read
    • Creates the batch with that key, which is the step that drives the rate limiter's internal file read; this is the call that currently hangs past the harness's 60s read timeout and is the reason for the skip
    • Polls the spend logs for the key until at least one row lands, so the assertion runs against flushed state rather than an empty table
    • Re-reads the same window and asserts no unattributed row appeared that was absent from the snapshot, which is the LIT-3266 contract: the limiter's internal read must carry the batch's auth metadata instead of spawning a row with empty api_key/user
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

The batch rate limiter counts input tokens by awaiting litellm.afile_content
with no timeout, so a slow Files API holds POST /v1/batches open past any
client deadline; stage saw 63.6s against the harness's 60s read timeout. The
test times out before reaching the unattributed-spend-row assertion it exists
to guard, so it reports an infrastructure hang rather than the contract.

Skipping keeps the signal honest until the fetch is bounded.
@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Temporarily disables the hanging batch rate-limiter spend-attribution end-to-end test with a ticket-linked explanation.

  • Adds an unconditional pytest.mark.skip referencing LIT-5027.
  • Leaves product code and all other tests unchanged.

Confidence Score: 5/5

The PR appears safe to merge as a narrowly scoped test quarantine with no product-runtime impact.

The only change skips a test that cannot currently reach its assertion because the exercised server path exceeds the client deadline, and the skip clearly records the tracked condition for removal.

Important Files Changed

Filename Overview
tests/e2e/batches/test_batches_e2e.py Adds a documented temporary skip to a known hanging end-to-end test; no actionable defect was identified in the change.

Reviews (1): Last reviewed commit: "test(e2e): skip the batch rate-limiter s..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ryan-crabbe-berri
ryan-crabbe-berri enabled auto-merge (squash) July 31, 2026 01:18
@ryan-crabbe-berri
ryan-crabbe-berri merged commit 8ccbc3e into litellm_internal_staging Jul 31, 2026
74 checks passed
@ryan-crabbe-berri
ryan-crabbe-berri deleted the litellm_skip_batch_rl_unattributed_spend_e2e branch July 31, 2026 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants