fix(spend): fold logs-tab total into the page query to avoid a separate COUNT(*) - #31423
Conversation
|
|
Greptile SummaryEliminates the standalone
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to a single endpoint, the response contract is unchanged, and all edge cases (in-range rows, empty table, out-of-range page) have explicit regression tests. The window-function substitution is mechanically correct: COUNT(*) OVER () always returns the full-match count on every row of the page result, so the hot path never fires a second round-trip. The out-of-range fallback (page > 1, empty data) reuses the same where_conditions variable that the old standalone count used, so filter parity is preserved. The total_count column is safely popped with a default before serialisation. Test coverage is thorough and the new tests fail on the pre-fix code as described. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/spend_tracking/spend_management_endpoints.py | Removes the standalone COUNT() round-trip for ui_view_spend_logs and replaces it with a COUNT() OVER () window function folded into the page query. The fallback to a direct count is correctly scoped to page > 1 with an empty result set; page 1 with no rows correctly returns zero without a count call. |
| tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py | Existing mocks updated to inject total_count into each returned row, matching the new window-function mechanism. The _reconstruct_ui_where_from_sql helper infers the active filter from the SQL text rather than from the now-absent count() call. Coverage is maintained and assertions remain strict on total/total_pages values. |
| tests/test_litellm/proxy/spend_tracking/test_spend_query_optimization.py | Three new tests added: verifies no separate COUNT(*) call fires on the hot path, verifies page 1 with an empty table returns zero without a count call, and verifies an out-of-range page (page > 1, empty result) falls back to a direct count so total/total_pages stay accurate. |
Reviews (4): Last reviewed commit: "fix(spend): fold logs-tab total into the..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
e016b85 to
921eb9e
Compare
…te COUNT(*) The spend-logs UI list endpoint (/spend/logs/ui, /spend/logs/v2) ran a standalone SELECT COUNT(*) before the page query to compute total_pages. On sharded engines like YugabyteDB a COUNT(*) is a distributed RPC that contacts every tablet leader and aggregates partial results regardless of row count, so it hits the distributed RPC timeout and the logs tab 500s even on a one-minute window with a couple of rows. The startTime range cannot prune tablets because rows hash to tablets on request_id, not startTime. Fold the count into the same scan as the page data with COUNT(*) OVER () and read total off the returned rows, dropping the helper column before serialisation. One distributed scan per page load instead of two; the response shape is unchanged. An empty page carries no count row, in which case the total is zero. Resolves LIT-4027
921eb9e to
cf34162
Compare
|
Addressed the out-of-range page edge case from the last review. The hot path (page 1 and in-range pages) still issues a single scan with no separate count; only an out-of-range page whose offset overshoots the last row now falls back to a direct count so total/total_pages stay accurate instead of collapsing to zero. Added a regression test for that fallback. @greptileai |
Relevant issues
Resolves LIT-4027
Linear ticket
LIT-4027
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
The logs tab (
/spend/logs/ui,/spend/logs/v2) ran a standaloneSELECT COUNT(*)before the page query to computetotal_pages. On a sharded engine like YugabyteDB aCOUNT(*)is a distributed RPC that contacts every tablet leader and aggregates partial results regardless of row count, so it hits the distributed RPC timeout and the tab 500s even on a one-minute window with a couple of rows. ThestartTimerange can't prune tablets because rows hash to tablets onrequest_id, notstartTime.Reproduced against a live proxy on real Postgres with
log_statement=all, after three realgpt-4.1-minichat completions populatedLiteLLM_SpendLogs. The customer-visible behavior is the SQL the endpoint emits per page load; on YugabyteDB that standalone count is the statement that times out.Before the fix, one logs-tab request emits a separate sub-select count statement (parse + bind + execute):
After the fix the standalone count is gone; the count rides along the single data scan via
COUNT(*) OVER (), and the HTTP response is identical:Statement tally across the two runs, isolated by a log marker:
The helper
total_countcolumn is stripped before serialisation, so it never leaks into the response rows (row0 has total_count key: Falseabove).Type
🐛 Bug Fix
Changes
litellm/proxy/spend_tracking/spend_management_endpoints.py: inui_view_spend_logs, drop theSpendLogsRepository(...).table.count(...)round trip and addCOUNT(*) OVER () AS total_countto the existing page query.total_recordsis read off the first returned row and thetotal_countcolumn is popped from each row before it reaches_build_ui_spend_logs_response. One distributed scan per page load instead of two; the response fields (data,total,page,page_size,total_pages) are unchanged.The hot path that the ticket reports (the first logs-tab load, page 1, and any in-range page) always returns rows, so the count rides along on those rows and the standalone count is gone. The one case with no count row is an out-of-range page whose offset overshoots the last matching row; there
totalwould otherwise collapse to zero, so that single case falls back to a direct count to keeptotal/total_pagesaccurate. That fallback never fires on the reported path, so the YugabyteDB timeout stays removed.This keeps the scope to the logs-tab list endpoint that the ticket reports. The session drill-down endpoint
ui_view_session_spend_logshas the sametable.countshape but filters on a single indexedsession_id, so it isn't the reported hot path; folding its count the same way is a reasonable follow-up if it ever shows up on YugabyteDB.Tests in
tests/test_litellm/proxy/spend_tracking/test_spend_query_optimization.pyassert that the endpoint issues no separate count call on the hot path, that the page SQL carriesCOUNT(*) OVER (), thattotal/total_pagesare derived from the row count, that the helper column doesn't leak, that a page-1 empty result reports a zero total without a count call, and that an out-of-range page falls back to a direct count so the total stays accurate. They fail on the pre-fix code (the separate count call fires) and pass with the fix. The existingtest_spend_management_endpoints.pymocks were updated to the new mechanism (the filter now derives from the single query and each row carriestotal_count)