Skip to content

core: unbounded MemWAL read fan-out — warn and expose a metric when a shard's pending generations pile up #259

Closed

Description

@beinan

Problem

StorageBase::wal_shard_snapshots() reads every shard's manifest and hands every flushed_generation to LsmScanner. Each generation is opened as its own Lance dataset on every get/list. There is no cap, no warning, and no metric.

In production one generic store reached ~12.6k pending generations across 20 shards (900–1,550 per shard) because its merge path was starved (#256). Every read opened all 12.6k, and workers OOMKilled at 32 GiB within ~6 minutes of restart for three days. Nothing in logs or metrics pointed at the store until the crash logs were read by hand: 99% of the final-minute log lines were lance::dataset_events loads of _mem_wal/<shard>/<gen> for that one store.

#256 fixes the drain, but the failure mode is reachable again through any other cause — a shard whose owner pod is gone (nobody can merge it), a writer that outpaces the merge, a misconfigured ROLLOUT_MERGE_AFTER_GENERATIONS = 0. One shard at ~1,500 generations is enough to take out every reader of the store.

Proposed

Small, no behaviour change:

  1. In wal_shard_snapshots() (or wherever ShardSnapshot is built), record flushed_generations.len() per shard into a histogram, e.g. rollout_wal_pending_generations{kind, store} (or a gauge keyed by store, if cardinality is acceptable — there are a few thousand stores per worker).
  2. warn! once per store per interval when any shard exceeds ROLLOUT_WAL_PENDING_WARN_GENERATIONS (default 256; 0 disables), with store, shard_id, pending, so the log is grep-able the way rollout_large_blob (feat(server): add blob-size histogram and oversized-blob warn log #240) is for oversized blobs.
  3. Optionally expose the per-store max in the GET /generic/{name} / experiment info responses so the master UI can show it.

Out of scope but related: a hard cap on how many generations a single read will open (fail the read or fall back to base-table-only with a warning) would turn an OOM into an error. That is a behaviour change and deserves its own discussion.

Refs #256, #257, #258.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions