Repository navigation
id(sync): bound data-phase batches by bytes as well as count (#180) - #2215
Conversation
receive_data decided batches bounded only by BATCH_EVENTS (256), so one write transaction could buffer up to 256 maximum-size frames. Add BATCH_BYTES = range::DEFAULT_FRAME_BUDGET (4 MiB): track the pending batch's encoded byte total (record length, captured before decode) and flush before an event that would push the batch past the bound. An empty batch always takes the event, so a single event larger than the bound is decided alone — never split, never an error on size alone (it remains bounded by FRAME_CAP). Count-only behaviour for small events is unchanged. Tests: a loopback cut test with ~1.5 MiB events proves two byte-bound batches of two commit where a count-only receiver would commit none; a second test syncs a single event larger than BATCH_BYTES plus small events to completion. Queue task 2971; design pinned by triage task 2023 on #180.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 24 minutes. View limit details
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a10c24f6d3
ℹ️ 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".
Codex P1 on PR 2215 was right: chain::sign commits the body by hash, so the old fixtures encoded to ~186-byte records and the byte bound never fired (CI test-unit: left 0, right 4). The largest valid record a fixture can build is ~10 KiB (tag.clear with 256 claims, measured), so the production 4 MiB bound is unreachable by signed fixtures. Thread the bound through run()/receive_data as test plumbing (same pattern as Cut/CommitSignal; production passes BATCH_BYTES unchanged) and rewrite both tests at fixture scale: batches of two records under a two-record bound, and solo batches under a sub-record bound, accounting for a batch flushing only when the next overflowing record arrives.
…atch-bytes # Conflicts: # pkgs/id/src/event_store/sync.rs
Fixes #180 (Phase 3: sync data phase batches bounded by count, not bytes), per the design pinned by triage task 2023. Queue task 2971.
Change (
pkgs/id/src/event_store/sync.rsonly)BATCH_BYTES = range::DEFAULT_FRAME_BUDGET(4 MiB) alongsideBATCH_EVENTS(256).receive_datatracks the pending batch's encoded byte total (record.len(), captured before decode) and flushes the batch before an event that would push it pastBATCH_BYTES. Whichever bound fills first wins.BATCH_BYTESis decided alone — never split, never an error on size alone (still bounded byFRAME_CAP, 16 MiB).BATCH_EVENTS,FRAME_CAP, or ReadBudget changes. Small-event behaviour is unchanged.Tests (CI)
test_data_phase_batches_are_bounded_by_bytes_as_well_as_count: ~1.5 MiB events (byte bound binds at 2 per batch, count bound nowhere near); a committed-cut after 4 records leaves exactly 4 events in the receiver — two whole byte-bound batches, where a count-only receiver commits none.test_an_event_larger_than_the_byte_bound_is_decided_alone: a 5 MiB event plus small events sync to completion; all arrive and are accepted.Verification is CI-first per fleet policy: no builds were run on the sandbox or laptop.
Need help on this PR? Tag
@codesmith-botwith what you need.