Skip to content

Single JSON serialization for DB and S3 - #4679

Merged
jmg-duarte merged 3 commits into
mainfrom
jmgd/single-auction-json
Jul 29, 2026
Merged

Single JSON serialization for DB and S3#4679
jmg-duarte merged 3 commits into
mainfrom
jmgd/single-auction-json

Conversation

@jmg-duarte

Copy link
Copy Markdown
Contributor

Description

Serialize the raw auction data a single data, instead of one per persistence method

Changes

  • Replaces the S3 and DB upload functions with a single one that serializes the JSON once

How to test

Ran it last night in mainnet prod, the dip around 22 was when it was deployed

Screenshot 2026-07-29 at 09-03-33 Edit panel - Critical Latency Metrics - Dashboards - Amazon Managed Grafana

@jmg-duarte
jmg-duarte requested a review from a team as a code owner July 29, 2026 08:21
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jmg-duarte's task in 2m 48s —— View job


PR Review: Single JSON serialization for DB and S3

  • Read changed files & diff
  • Analyze correctness/behavior changes
  • Post inline review comments

Nice, clean refactor. Collapsing the two independent serializations (DB via to_string, S3 via uploadto_vec) into a single serde_json::to_vec shared as Bytes removes the previous double serialization and the two deep clones of the domain auction (from_domain now borrows and the auction is handed back to the caller). Behavior is preserved: DB write is unconditional, S3 upload still skipped for empty-order auctions, and the enqueue-before-S3 ordering is intact. The serialization-failure path is actually cleaner than before (fails fast without a phantom enqueue).

One trade-off worth being aware of (already acknowledged by the code comment): serialization now sits on the run loop's critical path, whereas before both serializations happened entirely in background tasks. The production graph and the serialize_auction overhead metric cover this, so it looks intentional and measured.

Only one minor, optional nit posted inline (Bytes::from vs Bytes::from_owner). Nothing blocking.

Comment thread crates/autopilot/src/infra/persistence/mod.rs Outdated

@MartinquaXD MartinquaXD 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.

The idea makes sense to me but the current implementation leaves some performance on the table IIUC.

})
.await
.context("auction serialization task panicked")??;
let data = std::str::from_utf8(&json).context("serialized auction is not valid UTF-8")?;

@MartinquaXD MartinquaXD Jul 29, 2026

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.

I think we could avoid re-checking that this is valid utf8 by passing the AuctionUpload struct directly and making its constructor do the serialization. That way we know the immutable bytes we get from the AuctionUpload struct is not only valid utf8 but even valid JSON.
In the end we can just cast the bytes to a string in the DB query IIRC. At the very least that should at least move the re-validation into postgres if it doesn't skip it outright.

Comment thread crates/autopilot/src/infra/persistence/mod.rs Outdated
@jmg-duarte jmg-duarte added the hotfix Labels PRs that should be applied into production right away label Jul 29, 2026
@jmg-duarte
jmg-duarte added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 3c4bb41 Jul 29, 2026
22 checks passed
@jmg-duarte
jmg-duarte deleted the jmgd/single-auction-json branch July 29, 2026 16:30
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

hotfix Labels PRs that should be applied into production right away

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants