Skip to content

fix(ops): #60 cap + observe queryOpSet fan-out size - #247

Open
JustMaier wants to merge 2 commits into
mainfrom
fix/query-op-set-cap
Open

fix(ops): #60 cap + observe queryOpSet fan-out size#247
JustMaier wants to merge 2 commits into
mainfrom
fix/query-op-set-cap

Conversation

@JustMaier

Copy link
Copy Markdown
Contributor

Summary

  • Adds bitdex_query_op_set_fanout_size histogram + bitdex_query_op_set_rejected_total counter + bitdex_query_op_set_applied_slots_total counter so we can size a real cap from prod data instead of guessing.
  • Adds BITDEX_QUERY_OP_SET_MAX_FANOUT env var. Default usize::MAX — ships with no enforcement so the histogram populates from real traffic. Operator sets a finite value (manifest tweak + roll) once the distribution is in.
  • Skips the op (with tracing::warn! + counter increment) when fan-out > cap. Cursor advances; better than blocking the WAL reader forever or OOM'ing.
  • Drive-by: src/relay/auth.rs test helper &str → &'static str to unblock cargo test --lib (separate commit 8fa2cd4).

Why

apply_query_op_set is invoked from the WAL reader thread to apply nested ops to every slot matching a queryOpSet's filter. The matching path:

let query = BitdexQuery {
    filters,
    sort: None,
    limit: usize::MAX,           // unbounded
    skip_cache: true,            // cold every call
    ...
};
let result = engine.execute_query(&query)?;
let slot_ids = &result.ids;
for &slot_id in slot_ids { ... per-slot ops ... }

No cap on result.ids length. Under burst load + prod's _RJEM_MALLOC_CONF=dirty_decay_ms:0,muzzy_decay_ms:0, each cold execute_query allocation is held in resident memory, ratcheting RSS. 2026-04-29 measurement: identical 50-fan-out burst on local rig produced +820 MB sustained, no decay with prod MALLOC_CONF vs -60 MB decay under default jemalloc settings. That's the smoking gun behind the v1.0.177-jemalloc shadow-on OOM (RSS 11.7 → 110 GB in <30s on talos-wjh-tgy).

#61 (jemalloc decay tuning) closes the allocator-side half of the loop. This PR closes the per-call workload half.

What's new

Metric Type Labels
bitdex_query_op_set_fanout_size Histogram index
bitdex_query_op_set_rejected_total Counter index, reason
bitdex_query_op_set_applied_slots_total Counter index
BITDEX_QUERY_OP_SET_MAX_FANOUT Env var default usize::MAX

Histogram buckets: [1, 10, 100, 1K, 10K, 100K, 1M, 10M, 100M].

Skip path emits:

WARN ops_processor: queryOpSet '<query>' matches <N> slots, exceeds cap <C> — op skipped (data drift)

Trade: skip semantics

When cap is exceeded the op is skipped silently (log + metric, no error to /ops POSTer). Cursor still advances. Drift consequence: the trigger that produced this queryOpSet (e.g. Post.publishedAt update fanning out to N Images) misses some rows. Acceptable because:

  • Worst case: pop publishedAt update misses a few popular MVs briefly — re-runnable via reload.
  • Better than blocking the WAL reader forever (cursor stalled = ALL ops blocked).
  • Better than crashing the pod under memory pressure.

#62 (paginate-instead-of-skip) is the proper fix once we have a histogram-driven cap value.

Alert recommendation

rate(bitdex_query_op_set_rejected_total{reason="fanout_too_wide"}[5m]) > 0

Page when rejection rate > 0. Drift is invisible without this.

Test plan

  • cargo check --features server,pg-sync --bin bitdex-server passes.
  • test_max_fanout_default_is_usize_max — unset env yields usize::MAX.
  • test_max_fanout_env_override_parsesBITDEX_QUERY_OP_SET_MAX_FANOUT=100000 yields 100K.
  • test_max_fanout_invalid_env_falls_back_to_default — non-numeric env falls back to default.
  • End-to-end cap-exceed path through real ConcurrentEngine — deferred to docs: generational cache invalidation design #62 since it requires the same fixture work paginate-instead-of-skip will need anyway.
  • Histogram observation verified live once perf: generational doc cache eviction #61 ships and prod broadcasts data.

Refs

🤖 Generated with Claude Code

JustMaier and others added 2 commits April 29, 2026 15:56
Pre-existing test-only rot (not introduced by any recent change). E0521
"borrowed data escapes outside of function" because axum's HeaderMap::insert
requires a 'static key. All callers pass string literals, so widening the
parameter to &'static str is the smallest fix.

Caught while running cargo test --lib for an unrelated PR — the lib test
target was failing to build, masking real test signal. This unblocks the
test suite without touching production code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The WAL reader's apply_query_op_set runs `engine.execute_query` with
`limit: usize::MAX` and `skip_cache: true`, then iterates over every
matching slot to apply the nested ops. There is no cap on result.ids size
and no concurrency gate, because the WAL reader is single-threaded — but
under burst workload the per-call transient allocations (result Vec,
roaring slot iteration, batched bitmap mutations) can drive RSS sharply
under jemalloc's `dirty_decay_ms:0` prod config (issue #60 / #61
investigation, 2026-04-29).

This change adds:

  - `bitdex_query_op_set_fanout_size` HistogramVec — observed BEFORE the
    cap so we capture the would-be-rejected upper tail. Buckets span
    1 → 100M, sized for the 22.9M nsfwLevel-eq match seen during the
    OOM repro.
  - `bitdex_query_op_set_rejected_total` IntCounterVec, label
    `reason="fanout_too_wide"`. Alert when rate > 0 — every rejection
    is a missed mutation (data drift) until #62 (paginate-instead-of-skip)
    lands.
  - `bitdex_query_op_set_applied_slots_total` IntCounterVec — cumulative
    slot mutations, lets us distinguish narrow (postId-eq) from wide
    fan-outs at the work-unit level.
  - `BITDEX_QUERY_OP_SET_MAX_FANOUT` env var. Default `usize::MAX`
    (effectively no cap) so the histogram gathers prod data first.
    Operators flip a finite value once the distribution is in.
  - `metrics_bridge_handle()` accessor on ConcurrentEngine so paths
    outside the engine (here, ops_processor::apply_query_op_set) can
    read the bridge optionally — `None` in tests + dump-only contexts.

Trade: when cap is exceeded the op is skipped silently (log + metric
only). Cursor advances. Better than blocking the WAL reader forever or
letting OOM happen. #62 paginate-instead-of-skip is the proper fix.

Tests cover env parsing — default, override, malformed. The end-to-end
cap path is tested via the existing apply_query_op_set route in #244's
integration test once a real engine fixture is wired in (deferred to
#62 since it requires a fresh-data dir + worktree dependency).

Refs: scarlet's design, 2026-04-29 ratchet-vs-decay measurement
(MALLOC_CONF dirty_decay_ms:0 → RSS +820MB sustained vs default decay
-60MB on identical workload). Pairs with #61 (jemalloc decay tuning).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JustMaier

Copy link
Copy Markdown
Contributor Author

Code review passed (Scarlet, via team channel). Histogram observed before cap check captures rejected upper tail. Default usize::MAX ships unenforced — populates histogram first, operator tunes from real distribution after 24h soak. Skip-on-overflow with cursor-advance is correct (acknowledged data drift, tracked as #62 paginate-instead-of-skip). 174 net lines, 5 files. 3/3 unit tests pass, cargo check passes. Ready to merge after scope decision (#61 manifest land + tonight relay-mode flip vs full server-flip).

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.

1 participant