Skip to content

feat: support zstd response encoding based on Accept-Encoding#62

Open
mo4islona wants to merge 1 commit intomasterfrom
fix/zstd-response-encoding
Open

feat: support zstd response encoding based on Accept-Encoding#62
mo4islona wants to merge 1 commit intomasterfrom
fix/zstd-response-encoding

Conversation

@mo4islona
Copy link
Contributor

@mo4islona mo4islona commented Mar 25, 2026

Summary

  • Respect Accept-Encoding header per RFC 7231 §5.3.4
  • Accept-Encoding: zstd → respond with zstd compression
  • Accept-Encoding: gzip → respond with gzip (backward compatible)
  • Quality values supported: zstd;q=1.0, gzip;q=0.5 → zstd wins
  • At equal q, prefer zstd (faster decompression, better ratio)
  • Default to gzip when no Accept-Encoding header present

Changes

  • New crates/hotblocks/src/encoding.rs: ContentEncoding enum with RFC-compliant parser (13 unit tests)
  • api.rs: extract Accept-Encoding from request, pass through to query pipeline
  • running.rs: Compressor enum wrapping GzEncoder/ZstdEncoder, selected per-request
  • response.rs, service.rs: plumb encoding through query pipeline

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 25, 2026 08:12
@mo4islona mo4islona requested a review from kalabukdima March 25, 2026 08:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds per-request response compression negotiation so the hotblocks service can return zstd or gzip depending on the client’s Accept-Encoding.

Changes:

  • Introduces ContentEncoding and an Accept-Encoding parser with unit tests.
  • Plumbs selected encoding from the HTTP layer through the query pipeline.
  • Adds a runtime compressor abstraction to emit either gzip or zstd streams.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
crates/hotblocks/src/encoding.rs New ContentEncoding type and Accept-Encoding parsing + tests.
crates/hotblocks/src/api.rs Extracts Accept-Encoding, parses request body manually, sets content-encoding.
crates/hotblocks/src/query/service.rs Adds encoding parameter to query entrypoints and passes it to QueryResponse.
crates/hotblocks/src/query/response.rs Passes encoding into RunningQuery::new.
crates/hotblocks/src/query/running.rs Implements Compressor enum wrapping gzip/zstd encoders.
crates/hotblocks/src/main.rs Registers new encoding module.
crates/hotblocks/Cargo.toml Adds zstd dependency.
Cargo.lock Locks zstd dependency.
crates/hotblocks/src/query/mod.rs Minor formatting-only change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +48 to +52
if let Some(enc) = encoding {
if q > best_q || (q == best_q && enc == ContentEncoding::Zstd) {
best_q = q;
best = Some(enc);
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

Tie-breaking uses q == best_q on f32, which can be unreliable due to floating point representation (two decimal strings that should be equal may compare unequal after parsing). To make tie-breaking deterministic, consider parsing q into an integer scale (e.g. 0..=1000) or using an epsilon/ordered representation.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

RFC q-values have at most 3 decimal digits. f32 represents all values with ≤3 decimal digits exactly. Not a practical concern.

Comment on lines +200 to +209
async fn extract_encoding_and_body<T: serde::de::DeserializeOwned>(req: Request) -> Result<(ContentEncoding, T), Response> {
let accept = req.headers()
.get("accept-encoding")
.and_then(|v| v.to_str().ok());
let encoding = ContentEncoding::from_accept_encoding(accept)
.unwrap_or(ContentEncoding::Gzip);
let body = axum::body::to_bytes(req.into_body(), 1024 * 1024).await
.map_err(|_| text!(StatusCode::BAD_REQUEST, "failed to read body"))?;
let query: T = serde_json::from_slice(&body)
.map_err(|e| text!(StatusCode::BAD_REQUEST, "{}", e))?;
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

extract_encoding_and_body hardcodes a 1 MiB body limit and no longer enforces Content-Type: application/json (unlike Axum’s Json extractor). This is a behavioral regression for large queries and can accept unexpected content types. Consider keeping the previous JSON extractor semantics (or at least matching its default limit / adding an explicit Content-Type check).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed — reverted to Axum's Json extractor. Headers parsed separately via HeaderMap.

@mo4islona mo4islona force-pushed the fix/zstd-response-encoding branch from d7e958d to 84fa936 Compare March 25, 2026 11:42
Respect client Accept-Encoding header per RFC 7231:
- If client sends Accept-Encoding: zstd → respond with zstd compression
- If client sends Accept-Encoding: gzip → respond with gzip (backward compat)
- Quality values respected (e.g. "zstd;q=1.0, gzip;q=0.5" → zstd)
- At equal q, prefer zstd over gzip
- Default to gzip when no Accept-Encoding header present

Related: subsquid/squid-sdk#456
@mo4islona mo4islona force-pushed the fix/zstd-response-encoding branch from 84fa936 to cfbd04c Compare March 25, 2026 11:42
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