Skip to content

stream.batch bounds members and per-member fences but not their product: 100,000 fence reads fit in one writer transaction #2606

Description

@oceanwaves630

Summary

stream.batch bounds its member count at 1,000 and each member's fence list at 100 entries, but
nothing bounds their product. A single call can therefore carry 100,000 fence reads, each a SQL
SELECT, all executed while the batch writer transaction is held.

What is already bounded, so the residual is clear

  • MAX_BATCH_MEMBERS = 1000 (crates/khive-pack-kg/src/handlers/stream.rs)
  • MAX_BATCH_OBSERVED = 100 for the batch-level observation set
  • MAX_NOTE_FENCES = 100 per fence list (crates/khive-runtime/src/note_write.rs), enforced by
    NoteFences::validate_count before the writer transaction opens, which is what fix(kg): bound the fence list before the writer transaction opens #2560 landed
  • The batch's own top-level fence is refused outright if list-valued — batch_fence requires one
    object

Each of those caps is sound on its own axis. What is missing is the cap on the axis they multiply
along: a member's fence list is per-member, and members are per-call.

Why it matters

The comment on MAX_NOTE_FENCES's refusal message states the reason the cap exists: "each fence is
a read taken while holding the writer". That reasoning applies unchanged to the aggregate, and the
aggregate is two orders of magnitude larger than the per-list number the cap was chosen against.

The effect is on competing writers, not on the caller: the caller waits for its own request, while
every other writer waits behind a transaction whose hold time scales with an input nobody bounded.

Expected

A cap on total fence entries per call, refused before the writer transaction opens, alongside the
existing per-list check rather than replacing it. The two checks answer different questions and the
per-list one gives a better error for the common mistake.

State the number and the reasoning behind it in the refusal message, the way the existing one does.

Acceptance

  • A call with 1,000 members each carrying 100 fences is refused, and the refusal names the total and
    the cap.
  • A call at exactly the cap is accepted, so the boundary is tested from both sides.
  • The refusal happens before any writer transaction is opened, which is the property the per-list
    check already has and the reason it is where it is.
  • Mutation control, stated before running: moving the aggregate check to after the transaction opens
    leaves the refusal arms green and only the before-the-writer arm red, which is what proves that arm
    tests placement rather than arithmetic.

Provenance note

This came from re-reading a review verdict on a merged PR whose blocking finding claimed the fence
list had no entry-count limit at all. That claim is false at current main — the limit exists and
is enforced ahead of the transaction. The aggregate is what survives re-derivation, so this issue is
filed for the residual rather than the original finding.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions