basecamp / basecamp/basecamp-sdk

Event feed: the live buffer bounds slots, not bytes, so the published ceiling is ~10 GiB at defaults

Open
#801 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
49
Forks
12
Avg merge
20h 47m
Merged PRs (30d)
89

Description

Raised by daybreak while reviewing #705/#778. Filed as release-blocking because
it is a decision about a PUBLISHED default that is normative for all six SDKs,
not a Go-local fix.

### The number

SPEC §23 publishes the connector's worst-case memory as

(pump depth + 2 + EVENT_FEED_LIVE_BUFFER_CAPACITY) × EVENT_FEED_MAX_FRAME_BYTES

which at the shipped defaults — 256, 10,000, and 1 MiB — is **≈ 10 GiB**. The
spec already says so, and says it is "reached only if every slot holds a
maximum-size frame". That parenthetical is doing a lot of work, and this issue
is about whether it is doing too much.

### Why it is not obviously fine

The live buffer evicts by COUNT, not by bytes. Nothing in the connector limits
the total bytes retained: 10,000 slots is 10,000 slots whether each holds a
600-byte `message.created` or a 1 MiB frame. The party choosing which is the
PEER — the cable server — and the connector is a client library whose whole
transport layer is a documented extension point, so "the peer is our own
server" is an assumption about deployment rather than a property of the code.

The realistic figure is nowhere near the ceiling: real event frames run a few
hundred bytes, so a full buffer is ~10 MB. The gap between ~10 MB expected and
~10 GiB permitted is the problem — it is three orders of magnitude of headroom
that no operator asked for and no default limits.

### Options

1. **A default byte cap on the live buffer**, with eviction routed through the
existing `buffer_overflow` signal. §23 already sanctions this — "Implementations
MAY additionally impose a total byte cap on the live buffer; if they do,
eviction routes through the same overflow signal, never a silent drop" — so
the mechanism and its observability are already specified. This makes it a
MUST with a named constant (`EVENT_FEED_LIVE_BUFFER_BYTES`, 64 MiB would put
the ceiling near 128 MiB with the queue) rather than an option nobody took.
Recommended: it bounds the thing that is actually unbounded, and it needs no
new signal, no new state, and no new observer callback.
2. **Lower `EVENT_FEED_LIVE_BUFFER_CAPACITY`.** Cheapest, and wrong: it trades
the retention the ownership cut depends on for a bound it only approximates,
and it still scales with `MAX_FRAME_BYTES`.
3. **Accept and document.** Defensible only if the answer to "who chooses frame
size" is "an operator who also chose `MAX_FRAME_BYTES`". It is not: the
default is 1 MiB and the peer picks each frame's actual size underneath it.

### Acceptance criteria (for option 1)

1. A named constant in §23's Appendix A table with the other `EVENT_FEED_*`
rows, and the ceiling formula restated in terms of it.
2. Eviction on the byte bound raises the SAME `buffer_overflow` signal with the
same disposition contract — no second signal kind, no silent drop.
3. A test that fills the buffer with large frames and asserts eviction happens
on BYTES before the count bound is reached, and that the signal carries the
dropped count.
4. The option remains configurable, and setting it must not be able to make the
buffer smaller than one frame.
5. Because the ceiling is published, `SPEC.md` and the three code comments that
restate the formula move together — there is a `make doc-constants-check`
convention for exactly this class of restatement.

### Scope note

This is deliberately NOT being folded into the Event Feed connector PR stack
(#777/#705/#778). Those ship the Go reference against the ceiling as currently
published; changing the ceiling changes a cross-SDK constant and belongs with
the SDK-wide rollout, not inside the branch that is trying to land the
reference implementation.

Contributor guide

Open the contributing guide

Research direction

Start with SPEC.md §23 and its Appendix A constant table, then inspect the three code comments that restate the ceiling and the existing buffer_overflow path. Run make doc-constants-check and trace how the live buffer reports eviction. Done means a configurable byte cap, byte-triggered eviction through the same signal with the dropped count, one-frame minimum behavior, and synchronized specification and comment updates.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.