Skip to content

fix(py): compress replica writes that carry their own credentials - #3448

Open
Emil F (emil-lc) wants to merge 1 commit into
mainfrom
emil/multi-replica-memory-3
Open

fix(py): compress replica writes that carry their own credentials#3448
Emil F (emil-lc) wants to merge 1 commit into
mainfrom
emil/multi-replica-memory-3

Conversation

@emil-lc

@emil-lc Emil F (emil-lc) commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

_create_run and _update_run took the compressed (zstd) path only when all six
per-call auth arguments were None. Any replica carrying e.g. api_url/auth fell through to
the uncompressed tracing_queue, silently. Using LANGSMITH_RUNS_ENDPOINTS
always produces that shape, so the whole documented multi-endpoint configuration paid the
uncompressed cost.

This was no accident - a zstd frame had nowhere to record where it should be POSTed.

With this PR the frame now carries the missing field. CompressedTraces.destinations records where a
frame goes, accepts() admits only ops bound for the same place, and the sender routes by
it (None keeps the old _write_api_urls behaviour). The check, the commit and the write
happen in one hold of the existing lock.

There is still one frame per client, so when a client has several destination sets only one is
compressed: the first to write owns it, and reset() deliberately does not clear
destinations, so ownership is decided once instead of re-raced after every flush. The
rest use the plaintext queue, exactly as today - nothing is worse than before. A one-time warning
replaces the silence.

create_feedback is the third writer into that frame and gets the same check.

Release Note

Replicas hitting the same destination URL now use compressed trace ingestion instead of silently falling back to plaintext. Payloads sent to different projects are bundled in the same zstd Multipart frame, greatly reducing the memory footprint (e.g 8 replicas go from ~90MB -> ~2MB)

Test Plan

  • python/tests/unit_tests/test_replica_compression.py (new, 25 tests): no cross-destination bleed with decompressed frame bodies, stable ownership across flushes, feedback kept out of a replica-owned frame, concurrent admission vs. drain
  • Credential resolution produces byte-identical headers to the previous paths on all 7 auth shapes, including the None-key and service_key-only cases
  • Non-regression: api_urls={A,B} still replays one frame to both; project-only replicas unchanged; existing test_multiple_endpoints.py / test_replica_endpoints.py untouched and passing
  • Full unit suite: 2047 passed (one pre-existing failure from a locally-set LANGSMITH_ENDPOINT; passes when cleared)

@codspeed-hq

codspeed-hq Bot commented Aug 26, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 10.89%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 26 untouched benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory test_bench[create_10_000_run_trees] 17.7 MB 16 MB +10.89%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing emil/multi-replica-memory-3 (0940dc2) with main (1228e5b)

Open in CodSpeed

@open-swe open-swe Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Open SWE Review: No issues found

Open SWE reviewed this PR and found no potential bugs to report.

Open in WebView Open SWE trace

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.

1 participant