Skip to content

mq: follow-ups from #624 — membership cost, Unowned batching, crash-restart wait, allocations, docs, per-tenant budgets #693

Open

Description

@EricAndrechek

Area: mq — external NATS, deferred follow-ups

#624's own change description called out several items as deliberately left for later. Each is described here on its own so it can be picked up independently; none of them block #624, and none were independently re-measured beyond what is noted.

1. Membership read cost. claimLoop.readMembers (internal/ingest/claims.go:367) reads every membership slot's lease every tick (coord.RetryPeriod, 2s by default) — one Held lookup per slot, up to membershipReaders (16, claims.go:92) in flight at once, capped at min(units, 64) slots. At the shipped default topology this is a bounded, modest read volume, but it grows with the configured slot count, and a failed read skips acting on the whole tick (tick's doc comment: "acting on a partial view of the members would move units for nothing"). Whether this read pattern is cheap enough at larger slot counts, and whether a failed read skipping the entire tick causes visible reassignment lag under a flaky coordinator, has not been measured.

2. Batching Unowned. ExternalNATS.Unowned (internal/mq/external.go:824) loops over every configured and extra unit and issues one e.js.Consumer(ctx, ...) API round trip per unit, sequentially, to read each durable's ConsumerInfo. This is called periodically (per the wavehouse_ingest_shards_unowned gauge, read every 15s by the lowest-ranked member only, per deployment.md). At larger V (shards per partition) this is N×V sequential round trips per read; batching them (concurrently, or via a JetStream API that can answer for several consumers at once) was left as a follow-up rather than done in #624.

3. A lone process's crash-restart wait. The original design note for this item said a lone process, after crashing and restarting, "gets the dead run's rows back after ack_wait, not at once." Reading the current code (internal/ingest/claims.go's take, and internal/mq/external.go's ResetOrphaned/recentlyActive) suggests this may be more favorable than that: take marks every target unit "orphaned" immediately when a process finds itself alone (alone := l.prev == nil && len(live) == 1 && live[0] == l.slot), then retries ResetOrphaned every tick (2s) until either it succeeds or orphanWait (30s, claims.go:96) elapses, at which point it proceeds to bind anyway. Since ResetOrphaned typically succeeds once the crashed run's old pin has lapsed and it is no longer "recently active" — both keyed to PinnedTTL/minPinnedTTL, on the order of 10s — the wait in practice looks closer to ~10–30s than the full ack_wait default of 60s. This has not been measured end-to-end, so whether the original concern still applies (or in what scenario it still would) is unconfirmed.

4. Allocations on the hot ownership/delivery path. Noted as worth trimming in the change description; no specific allocation was pointed at, and none was independently profiled here.

5. The --coord-bucket docs pointer. The change description flagged that wavehouse mq permissions/mq manifests's --coord-bucket flag needed a docs pointer. Current docs/src/content/docs/deployment.md:357 and configuration.mdx:90 both already document --coord-bucket and its relationship to coord.nats.bucket, so this item appears to already be addressed — worth confirming rather than re-doing.

6. Per-tenant budgets under the nats backend (deferred when #624 was designed). The decision recorded then: "Partition/shard key is tenant+table (consistent hash); a flooding tenant touches every partition; per-tenant budgets are a later follow-up. Accepted." Today, mq.max_bytes_gb (the settings-directory per-tenant queue budget) is explicitly not applied under mq.backend: nats — a boot-time warning says so (internal/config/mq_nats.go's natsWarnings: "mq.max_bytes_gb (settings directory) is not applied with mq.backend=nats: a tenant's queue is bounded by its partition stream's limits, which are the operator's"). A tenant publishing heavily therefore consumes shared partition capacity with no per-tenant cap, by design, until this is picked up.

Found in review of #624.

Related: #624, #613.

Activity

  1. taitelee commented on Oct 12, 2026

    @taitelee
    Member

    Item 5 is already addressed. --coord-bucket and its relationship to coord.nats.bucket are documented in docs/src/content/docs/configuration.mdx (line 90) and docs/src/content/docs/deployment.md (lines 354 and 392). Verified against main at c01b912; nothing to do.

  2. taitelee commented on Oct 12, 2026

    @taitelee
    Member

    This is to be excluded in version 0.2.0.

    The reasoning are as follows:

    Items 1, 2 and 4 are performance follow up: membership reads cost under a millisecond per tick against a server with two orders of magnitude of headroom, Unowned stays well under its budget past anything near the shipped topology, and our delivery path allocates about 27 objects per row next to a NATS client that allocates far more.

    Item 5 is already done.

    Item 6 is the shared-partition design we recorded with #624, and the measured failure mode, one flooding tenant turning into 503s for its partition, is the accepted trade-off until per-tenant budgets get designed.

    Item 3 is the only correctness issue, and it is really #703: a per-table order inversion that needs a crash, not a clean stop, to trigger. How we guarantee order across a crash belongs with the work-sharding and Kafka research planned in the near future.

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

    enhancementNew feature or request

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions