perf(spend): move cost-callback payload deepcopy off the request event loop

[yassin-berriai]

Relevant issues

Linear ticket

Resolves LIT-4088

Pre-Submission checklist

• 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 requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

Live-proxy spend-tracking parity verification to follow in a comment

Type

🚄 Infrastructure

Changes

_PROXY_track_cost_callback persists spend by awaiting DBSpendUpdateWriter.update_database, which ran three synchronous copy.deepcopy calls on the request event loop before deferring the table updates to a background task. copy.deepcopy is pure-Python and GIL-bound, so each copy blocked the loop; under load this was part of the multi-second update_database a customer measured

Every consumer of the payload is read-only: the daily-spend helpers construct new dicts, _update_tag_db parses request_tags read-only, and _insert_spend_log_to_db only takes a lock and appends the reference to the in-memory queue that is later serialized read-only. So the spend-log insert now receives the payload directly with no copy, and the single deepcopy the daily helpers need is taken at the top of _batch_database_updates, which already runs inside the background asyncio.create_task. The awaited request path now performs no deepcopy

A regression test counts copy.deepcopy calls and asserts none occur before update_database returns, that the relocated copy still isolates the daily helpers from later mutation of the source payload, and the existing daily-agent isolation test is preserved

[yassin-berriai]

@greptileai

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[greptile-apps]

Greptile Summary

This PR moves spend-tracking payload copying out of the awaited request path. The main changes are:

• Passes the spend-log payload directly into _insert_spend_log_to_db
• Moves the remaining copy.deepcopy into _batch_database_updates
• Updates spend writer tests to verify request-path behavior and daily-helper isolation

Confidence Score: 5/5

The change is narrowly scoped to spend-update copy timing and preserves the intended background isolation behavior.

The modified code path is covered by targeted regression tests that check no request-path deepcopy occurs and that daily helper payload isolation is retained.

No specific files need additional attention.

T-Rex Logs

What T-Rex did

• Inspected the base artifact from proof 0 to capture the initial state before mutation, noting update_database_source_has_deepcopy=True, counts_before_return=3, spend_log_payload_identity_same=False, and daily_snapshot_isolated=True.
• Inspected the head artifact from proof 0 to capture the post-mutation state after background tick, noting update_database_source_has_deepcopy=False, counts_before_return=0, counts_after_background_tick=1, daily_snapshot_isolated=False, daily_payload_tags_after_source_mutation=['tag-before', 'tag-after-mutation'], daily_payload_metadata_after_source_mutation={'marker': 'after-mutation'}, and spend_log_payload_identity_same=True.

_{Ran code and verified through T-Rex}

Comments Outside Diff (1)

1. General comment

   Background deepcopy does not isolate daily spend helpers from post-return payload mutation

   • Bug
     • On head, update_database returns without deepcopying and passes the original payload reference to the spend-log insert as intended, but because _batch_database_updates deepcopies the same original payload only when the scheduled task later runs, mutations made to the payload after update_database returns are captured by the daily spend helper payloads. The validation mutated request_tags and metadata immediately after update_database returned; the background task then deepcopied those mutated values, so daily_snapshot_isolated=False.
   • Cause
     • db_spend_update_writer.py schedules _batch_database_updates with payload=payload, where payload is still the original mutable logging payload. The only deepcopy is deferred inside _batch_database_updates, creating a race window between update_database return and the background task's first instruction.
   • Fix
     • Preserve request-path performance while closing the mutation window, for example by scheduling the background task with a payload snapshot created before returning via a non-blocking/owned immutable representation, or by ensuring get_logging_payload/update_database constructs an owned payload object that is not externally mutable before it is handed to _batch_database_updates. The daily helpers need a snapshot of the payload state at update_database scheduling time, not at background task execution time.

   _{Ran code and verified through T-Rex}

_{Reviews (3): Last reviewed commit: "perf(spend): move cost-callback payload ..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[yassin-berriai]

Proof of fix (live proxy)

Ran this branch as a live proxy against real Postgres and the real OpenAI API (openai/gpt-4o-mini, real spend). The change removes all three copy.deepcopy calls from the awaited request path and takes the one copy the daily-spend helpers need inside the already-backgrounded _batch_database_updates. The proof that matters is parity: the spend log and the daily-spend tables must still be written with correct, intact, isolated payloads

Confirmed the running proxy loads this branch (no deepcopy on the request path, one inside the background task)

deepcopy in update_database body:        False
deepcopy in _batch_database_updates:     True

One real request, then the persisted spend log and daily-spend rows

$ curl -s $B/v1/chat/completions -H "Authorization: Bearer $K" \
    -d '{"model":"gpt-4o-mini","messages":[{"role":"user","content":"Reply with exactly: cost path ok"}]}'
response: cost path ok

# newest LiteLLM_SpendLogs row
request_id    | chatcmpl-DvzvbWoQSkd9UtI6zHNEWRSpFfIuC
model         | openai/gpt-4o-mini
spend         | 3.9e-06
call_type     | acompletion
metadata_head | {"status": null, "max_retries": 2, "batch_models": null, "usage_object": {"total...

# row counts before -> after the single request
spendlogs   4 -> 5
daily_user  3 -> 4
daily_tag     -> 6

The spend log row carries the full metadata payload and the daily user/tag aggregates advanced, so moving the copy off the request path did not drop or corrupt any of the backgrounded writes

[yassin-berriai]

On the "daily spend helper isolation" note: the flagged window requires payload to be mutated after update_database returns but before _batch_database_updates runs its copy.deepcopy. That mutation does not happen in the real call graph

payload is built locally inside update_database and, after the field assignments at the top, it is only handed to two consumers: the inline spend-log insert and the backgrounded _batch_database_updates. The spend-log path (_insert_spend_log_to_db -> the in-memory queue -> the batch flush) only locks and appends the reference, then serializes it read-only ({**entry} / json.dumps); it never mutates the dict. The daily helpers receive the deepcopy, and the org/end_user/agent variants build new dicts via {**payload_copy, ...} rather than mutating in place. Nothing else holds a reference. So the object is effectively frozen the moment update_database finishes building it, and a snapshot taken in the background task is identical to one taken synchronously

The T-Rex harness surfaced the window by injecting a mutation after return, which is a code path the production callers do not exercise. The new regression test plus the existing test_daily_agent_receives_deepcopied_payload encode the read-only/isolation contract, so if a future change introduces a mutating consumer it will fail there rather than silently corrupting a row

Keeping a synchronous snapshot on the awaited path is exactly the copy.deepcopy cost this PR removes, so reintroducing it would defeat the change for a window that is not reachable

@greptileai

[greptile-apps]

That's a thorough and convincing walkthrough of the call graph. You're right on all three counts:

1. payload is locally constructed in update_database and never escapes to a caller that could race-mutate it — the mutation the T-Rex harness injected is not reachable from any production caller.

2. All downstream consumers are read-only — _insert_spend_log_to_db appends the reference and later serializes it with {**entry} / json.dumps; the daily helpers get the deepcopy and only build new dicts via {**payload_copy, ...}. No in-place mutation anywhere.

3. Reintroducing a synchronous snapshot would be self-defeating — the entire point is to move that GIL-bound work off the awaited path.

The new test_update_database_does_not_deepcopy_on_request_path correctly encodes the contract: zero deepcopy calls on the awaited path, the batch task still produces an isolated snapshot, and mutation of the original after the task ran doesn't bleed through. That's the right place to catch a future regression if someone introduces a mutating consumer.

Fair point — withdrawing the concern.

[yassin-berriai]

@greptileai