Skip to content

fix(core): bound Tx.count, agree a group's size, and evict a partial tx (C3) - #363

Merged
vieiralucas merged 1 commit into
mainfrom
core/c3-tx-count-bound
Jul 28, 2026
Merged

fix(core): bound Tx.count, agree a group's size, and evict a partial tx (C3)#363
vieiralucas merged 1 commit into
mainfrom
core/c3-tx-count-bound

Conversation

@vieiralucas

@vieiralucas vieiralucas commented Jul 27, 2026

Copy link
Copy Markdown
Member

Tx { id, count } tells a receiver to hold a group's members until count of them are present. Nothing bounded count at the decode boundary and nothing ever gave up on a partial group, so a rewritten envelope pinned members in the buffer for the life of the replica — persisted by encode_state, and (since C6) counted among the ids the replica holds. Completeness was also read off buffer[idxs[0]].tx.count, whichever member landed first. Hardening: nothing hangs or panics today.

The cap is a protocol constant, enforced at both ends

  • ARCHITECTURE §Scope Constraints said "capped (default 1000)" and nothing enforced it. It cannot be a setting: both ends decide completeness from a declared count, so a node with its own cap emits groups every peer refuses. MAX_TX_MEMBERS now sits beside Tx.
  • The codec refuses 0 and anything past the cap — the members never enter the buffer, which is what persists.
  • apply refuses the same sizes for an op that reaches it without crossing the codec (in-process relay, SDK) — same answer at both seams, and refusing holds nothing.
  • tag_atomic leaves an oversized group untagged rather than tagged with a size whose refusal takes the whole framed batch: otherwise an oversized transaction becomes dropped ops, not a non-atomic one.

A group's size is what its members agree it is

  • tx_declared_count requires unanimity across the bucket; a bucket without it names no group and is never complete. Read off one member, a rewrite chooses when the group commits.

Eviction is the give-up path, not a repair rule

  • Document::evict_partial_transactions() untags every group still waiting and reports how many. A member that never arrives is indistinguishable from one still in flight, so how long to wait is the caller's policy — the core reads no clock.
  • Untags rather than discards, for C11's reason: the members are ops the replica holds and no peer will resend. Leaves C6/C9's id accounting untouched (applied or still buffered, never free to re-mint) and C1's readiness gate running.
  • Stated rather than hidden: a replica that never evicts does not converge with one that does.

What two cold-review passes changed

  • The first cut untagged a whole bucket the moment it looked unreachable. Whether it looks that way depends on which members have landed: m1(3), m2(3), m3(2) delivered m1,m2,m3 released all three, delivered m1,m3,m2 released two and held m2 against a bucket already given up on. One op set, two states — and stranding on 4 of 6 orders where the defect it replaced stranded on 2. Only a judgement on a member's own declared size is order-free.
  • The second pass caught the "more members than the count" shape being sold as fixed. A rewrite consistent across every member is, to a receiver, an honest group of the smaller size followed by a stray: it commits at that size over whichever members arrived first. Now pinned by a test asserting the two orders disagree, corrected in ARCHITECTURE/DECISIONS, and filed as C18.

Rejected

  • Verifying the group id against TxId::derive of its members: with unanimity the wrong-size commit is already gone, so it buys nothing state-observable while newly depending on no seam ever renumbering a seq under a live tag.

Filed, not bundled

  • C17 — nothing calls the eviction seam yet (server room-maintenance tick + client session own the policy).
  • C18 — a bucket resolved at a rewritten size, or one an unrelated op squatted into, commits the wrong members order-dependently. Wants a persisted resolved-key set; eviction converges it until then.

Gates

  • crates/core/tests/transaction.rs +16 cases (every shape reproduced first, a spliced snapshot for the presentation that never commits, a six-order sweep pinning nothing is released early); crates/core/tests/convergence.rs gains a malformed-count arm that fails at seed 0 against the bucket-wide rule, plus a rewritten-smaller arm.
  • Mutation-swept whole-workspace, --no-fail-fast, measured: no codec bound 2 · no refusal at apply 1 · count off the first member 4 · no mint cap 1 · eviction a no-op 10 · eviction discarding instead of untagging 11 · a committed member waved past the readiness gate 8. Unmutated tree 0.
  • cargo test --workspace green, cargo fmt --all clean, Miri clean on the touched suites.

Summary by cubic

Bound atomic transaction size to a 1000-member protocol constant, required unanimous group size across members, and added an eviction path to prevent unbounded buffering and order‑dependent commits. Docs now state the cap and the eviction policy. Addresses C3; follow-ups: C20 (policy to call eviction) and C21 (order-dependent commit of rewritten groups).

  • New Features

    • Added Document::evict_partial_transactions() to untag and apply all waiting groups; returns the number evicted. Leaves op IDs held; caller decides when to run it.
  • Bug Fixes

    • Introduced MAX_TX_MEMBERS = 1000 as a protocol constant. The codec refuses count of 0 or > cap; apply also refuses out-of-range counts for in-process callers; tag_atomic emits oversized groups untagged so ops still merge.
    • Group completeness now requires unanimous count across members; mixed counts never complete and wait for eviction.
    • Docs clarified: the cap is protocol-wide; a late member after eviction rejoins its original bucket; periodic eviction must be age-aware to avoid untagging honest groups; double-envelope/order-dependent cases remain and are covered by C21.

Written for commit a23329d. Summary will update on new commits.

Review in cubic

@vieiralucas
vieiralucas requested a review from Copilot July 27, 2026 23:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vieiralucas
vieiralucas requested a review from Copilot July 27, 2026 23:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…tx (C3)

A rewritten `count` pinned atomic-transaction members in the receiver's
buffer for the life of the replica, and `encode_state` handed them to the
next one. The 1000-member cap ARCHITECTURE named is now a protocol
constant enforced at the decode boundary, at the apply seam an in-process
caller reaches without crossing it, and at the mint (an oversized group is
emitted untagged). A bucket's size is what its members agree it is, so a
rewritten first arrival no longer decides when the group commits.

Everything a local judgement cannot separate — a member that never
arrives, a bucket resolved at a size rewritten on every member, two
envelopes of one op id — waits for `evict_partial_transactions`, which
untags rather than discards so the ops still merge and no id is freed.
Releasing a bucket that merely looks unreachable is arrival-order
dependent and was rejected for it.
@vieiralucas
vieiralucas force-pushed the core/c3-tx-count-bound branch from 8f0f1c8 to a23329d Compare July 28, 2026 00:29
@vieiralucas
vieiralucas requested a review from Copilot July 28, 2026 00:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vieiralucas
vieiralucas merged commit dec6628 into main Jul 28, 2026
18 checks passed
@vieiralucas
vieiralucas deleted the core/c3-tx-count-bound branch July 28, 2026 01:11
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