Carry staged files to the hub over the tab-to-worker content lane - #30
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @LucaCappelletti94, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 5 days and 5 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: LucaCappelletti94/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Reviewer's GuideThis PR adds a private, FIFO tab-to-worker content lane carrying JSON plus Blob attachments, stages and hashes files before mutations, atomically commits content bookkeeping with the naming row, and resolves bytes locally or through server tickets without changing the server sync protocol. Sequence diagram for tab-to-worker staged content commitsequenceDiagram
actor Tab
participant Lane as InternalContentLane
participant Hub as RelayHub
participant Archive as ContentArchive
participant Replica as WorkerReplica
participant Upload as ContentUpload
Tab->>Lane: post_internal(ContentFrame::Stage, Blob)
Lane->>Hub: FIFO Stage frame with Blob
Tab->>Replica: with_conn(row, file_id)
Replica->>Hub: mutation changeset
Hub->>Hub: changeset_blob_values(changeset)
Hub->>Archive: chunk_file(BlobSource, mime)
Hub->>Archive: commit_staged(manifest, row)
Archive->>Replica: put_manifest and enqueue
Archive->>Replica: apply_changeset and record_tab_watermark
Hub->>Upload: wake_content()
Sequence diagram for content resolutionsequenceDiagram
actor Tab
participant Lane as InternalContentLane
participant Hub as RelayHub
participant Archive as ContentArchive
participant Server
Tab->>Lane: post_internal(ContentFrame::Resolve, file_id)
Lane->>Hub: Resolve request
Hub->>Archive: resolve_connection(file_id)
alt local content available
Archive-->>Hub: Resolved::Local(bytes)
Hub-->>Lane: ResolveReply with Blob
else server ticket available
Archive->>Server: request_connection_or(Read)
Server-->>Archive: read URL
Archive-->>Hub: Resolved::Remote(url)
Hub-->>Lane: ResolveReply with URL
else unavailable or timeout
Archive-->>Hub: Resolved::Unavailable
Hub-->>Lane: ResolveReply unavailable
end
Lane-->>Tab: TabResolved
Flow diagram for staged content pairing and refusalflowchart TD
A[Tab hashes Blob to FileId]
B["post_internal(Stage, Blob)"]
C[Commit row mutation]
D[Hub pairs staged Blob with 32-byte changeset value]
E[Hub re-hashes and chunks Blob]
F{Identity matches?}
G[Atomic commit: manifest, outbox, row, watermark]
H[Wake content upload]
I[Reject mutation and roll back bookkeeping]
A --> B --> C --> D --> E --> F
F -->|yes| G --> H
F -->|no| I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41cdc0ce08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if tab.staged.len() >= MAX_STAGED_CONTENT { | ||
| tab.staged.pop_front(); | ||
| } |
There was a problem hiding this comment.
When a tab has more than eight stages outstanding—for example, concurrent TabContent::stage calls while connection serialization delays their mutations—this branch silently discards the oldest blob. Its later mutation is then handled as an ordinary mutation because take_staged finds nothing, so the row is committed and forwarded without a manifest or upload entry, leaving content permanently unavailable despite staging appearing successful. Apply backpressure or reject the incoming stage rather than evicting an unpaired entry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5041ab. A full staging buffer now refuses the incoming file and logs a warning instead of evicting the oldest, so every accepted stage survives to its pairing.
| let (answer, observed) = content | ||
| .resolve_connection( | ||
| worker, | ||
| FileId::from_bytes(file_id), | ||
| sleep_ms(RESOLVE_WAIT_MS), | ||
| ) | ||
| .await; |
There was a problem hiding this comment.
When the content server is slow or does not answer a read ticket, this await occupies the hub's sole event-handling task until the 15-second timeout. During that interval no queued mutations, pings, stages, or other tab requests are serviced, and an in-progress upload being driven by the same handler also stops being polled; repeated resolves for unknown IDs can therefore keep every attached tab stalled. Drive the ticket wait as resumable state while continuing to service the hub event queue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5041ab. The hub no longer parks the event loop on a ticket answer. ContentArchive::start_resolve_connection sends the request and hands back a PendingConnectionResolve, the hub queues it with a deadline, settles it in handle_worker_event through the routed route_connection_event, and sweeps expired waits on its cycle, so mutations and other resolves keep flowing while one ticket is outstanding.
| let buffer = JsFuture::from(blob.array_buffer()).await?; | ||
| Ok(Uint8Array::new(&buffer).to_vec()) |
There was a problem hiding this comment.
For a large blob, especially the supported Video class, array_buffer() materializes the entire file in JavaScript memory and to_vec() immediately copies it again into Wasm linear memory solely to compute the identity. This produces a peak allocation of roughly twice the file size in addition to the blob backing store and can terminate the tab for otherwise valid large uploads; hash streamed blob slices instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5041ab. stage now hashes through 4 MiB Blob.slice windows with FileIdHasher::update, so peak memory is one window and the digest is unchanged.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #30 +/- ##
==========================================
+ Coverage 81.24% 82.79% +1.55%
==========================================
Files 115 114 -1
Lines 25806 25247 -559
Branches 25806 25247 -559
==========================================
- Hits 20965 20903 -62
+ Misses 3630 3132 -498
- Partials 1211 1212 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Resolves now wait as hub state: the ticket request goes out, the cycle keeps serving tabs and mutations, and the answer arrives through the normal event path or an expiry sweep. Staged blobs hash in 4 MiB windows instead of whole-file copies, and a full staging buffer refuses the incoming file instead of evicting one a mutation may still pair with.
|
1 participant
A browser tab can now hand a file to the worker and have it reach the server together with the row that names it. The lane rides the existing tab-to-worker message transport as a new internal frame tag carrying a JSON control message and the bytes as an attached
Blob, so the server protocol and every non-content path stay untouched.Ordering is what makes pairing safe without a handshake per file. The tab posts the stage before committing the row, the port is FIFO, and the hub's shovel polls the internal lane first, so the hub always holds the blob by the time a mutation arrives. Pairing ignores table and column names entirely, scanning only the 32-byte blob values the changeset writes, and the hub re-hashes the staged bytes rather than trusting either declaration, so a row naming the wrong identity is refused and nothing uploads. The manifest, the outbox entry and the application row commit in one transaction through the client's bookkeeping-aware helper, and the commit tells the upload driver directly because a mutation that produces no upstream event would otherwise leave the entry asleep. Resolution answers wherever it arrives, from the worker's own store while a file is unsent, from a server ticket once it is uploaded, and as unavailable after both bounds.
A refused ticket retries on the content backoff, which is why the test fixture sleeps for real. A sleeper that resolves instantly turns that backoff into a busy loop that starves the worker, and the fixture caught a fake server minting a grant URL the write-endpoint split refuses, so both shapes are pinned in tests now.
Summary by Sourcery
Carry staged browser files over the worker content lane and commit, upload, and resolve them consistently with the mutations that reference them.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: