feat(responses): Add optional compression in responses store - #1182
s-akhtar-baig wants to merge 14 commits into
Conversation
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Summary: Clean, well-structured compression implementation with solid backward compatibility. Schema versioning, auto-detection on decode, and migration documentation are all sound. Two findings below.
| Severity | Count |
|---|---|
| Medium | 2 |
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
leseb
left a comment
There was a problem hiding this comment.
[P1] Conversations deployments are incorrectly forced through the Responses v2 migration.
The shared SCHEMA_VERSION:38 now applies to the generated *_unused_responses:112 table used by openai_conversations. Existing v1 databases therefore fail initialization even though the Conversations schema did not change. The migration guide only covers the configured Responses table. Schema versions should be separated, or the generated table must be migrated and tested.
[P1] Required CI fails after changing payload columns to BLOB.
Two compact integration tests still read response_object and messages as String (line 691:691, line 993:993). Both the integration and coverage jobs fail with TEXT is not compatible with BLOB. Read bytes and deserialize with from_slice.
[P2] Compression can create records that cannot be read back.
encode:93 accepts arbitrarily large JSON, while decode:119 rejects decompressed values over 256 MiB. Enforce the same limit before a compressed write succeeds.
[P2] Public store constructors bypass compression validation.
SqliteResponseStore::new:69 and PostgresResponseStore::new:126 accept public StoreCompressionConfig values without validation. Invalid levels then fail only on first write, while algorithm: none silently ignores a level. Validate in both constructors.
Also missing: the required OpenAI SDK integration coverage for changes under apis/src/openai/.
And we need a benchmark this implementation, here we have done locally:
A directional M2 Pro microbenchmark of the exact zstd level-3 path showed:
| Payload | Extra write CPU | Extra read CPU |
|---|---|---|
| 10 KB | 8–16 µs | 3–12 µs |
| 100 KB | 23–77 µs | 17–99 µs |
| 1 MB | 0.24–0.73 ms | 0.12–0.95 ms |
The range reflects highly compressible JSON versus low-redundancy content. Storage shrank to 2–75% of original size.
Important caveats:
- The store compresses three JSON fields independently, so costs accumulate.
- Compression runs synchronously on the async worker (compression.rs); large/concurrent writes may hurt p99 latency.
- Database I/O was excluded. Against inference latency, the per-request cost is probably small, but throughput impact remains unmeasured.
- With compression disabled—the default—there is effectively no zstd cost.
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
|
Thanks, @leseb, for reviewing this PR! I have added fixes and a benchmark. PTAL when you get a chance. Note that compression on the three columns still run synchronously. I'd have to look into the critical (write) section and investigate whether making asynchronous calls improve current performance. I'd like to do this exploratory work in a follow-up issue/PR pair as this PR is getting on the bigger side. But please let me know if you think it should be part of this PR. Thanks again! |
leseb
left a comment
There was a problem hiding this comment.
one last remaining P2: zstd decode and explicit-compaction encode still perform potentially large synchronous CPU work on async workers (compression.rs:110). Offload it or impose a much smaller inline size/level bound.
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
Signed-off-by: Shabana Baig <43451943+s-akhtar-baig@users.noreply.github.com>
|
@leseb, I have updated the encode and decode functions to be asynchronous. Please let me know if I missed anything. Thanks! |
Summary
Adds optional zstd compression for the OpenAI responses store. Compression applies only to the three payload columns on the responses table — response_object, input, and messages. Conversation and conversation-item payloads are unaffected and remain plain TEXT.
Major Changes
docs/store/schema-migration.md.Related issue
Closes #629
Validation
make lintChecklist
Signed-off-bytrailer.Breaking changes