Skip to content

fix(gorillas3): TSDB appender out-of-bounds with rotating-cardinality metrics - #354

Merged
zzylol merged 1 commit into
mainfrom
fix/gorillas3-tsdb-out-of-bounds
May 9, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/gorillas3-tsdb-out-of-bounds

Conversation

@zzylol

@zzylol zzylol commented May 9, 2026

Copy link
Copy Markdown
Contributor

Bug

Agent processor crashed 60s after start with:

2026-05-08T22:10:14.679Z error gorillas3processor/processor.go:325 gorillas3: tsdb block build failed
  error="tsdb appender.Append: out of bounds"
  at flushTSDB → flushWindow → Start.func1

Reproducible 100% of the time after PR #338's high-cardinality rotating metrics (unique_users_per_min, top_endpoint_qps) started flowing. Blocks the entire MVP demo.

Root cause (hypothesis (a) — timestamp ordering)

Prometheus TSDB's head.Appender() snapshots minValidTime = appendableMinValidTime() = max(MaxTime - chunkRange/2, headMinValid) at the time the appender is created (or, for an empty Head, on first Append). For a fresh BlockWriter Head this means minValidTime = firstAppendedSampleTs - chunkRange/2 — fixed for the appender's lifetime.

With a 60s tsdb_block_duration and Go's randomised map iteration over the window, if the first-visited series held samples in e.g. the [40..55s] slice of the window, the floor locked at 10s = 40 - 30. Any subsequent series with samples at t<10s then tripped storage.ErrOutOfBounds.

PR #338's unique_users_per_min rotates user_ids over the window — different user_id series have NON-OVERLAPPING per-series timestamp ranges by design — which made cross-series-ordering OOB fire on every flush.

Fix

opentelemetry-collector-contrib-patch/processor/gorillas3processor/

  1. tsdb_block_writer.go appendWindow: flatten every (labelset, ts, v) tuple across all series, sort globally by timestamp ascending, then drive app.Append. Guarantees the first appended sample is the smallest ts in the window, so minValidTime = min(window) - 30s sits below every other sample. (The per-series ascending sort is preserved as a stable secondary order.)
  2. Defensive: errors.Is(err, storage.ErrOutOfBounds) from Append is now NON-fatal — the sample is skipped, a counter is bumped, and the appender continues. The artifact surfaces a NumOOBDropped count.
  3. selfmonitor.go: new gorillas3_tsdb_oob_samples_dropped_total counter so operators see drift.
  4. processor.go flushTSDB: log a Warn (not Error) per-flush when drops occur; bump the counter.

Test coverage

tsdb_block_writer_test.go:

  • TestTSDBBlockBuilder_RotatingCardinalityNoOOB — 50 trials of randomised map iteration over 8 series with non-overlapping 7-second slices in a 60s window. Fails on the unfixed code with the exact error from production; passes on the fix every iteration.
  • TestFlushTSDB_OOBDoesNotCrashProcessor — drives the same scenario through the full ConsumeMetrics → flushWindow → flushTSDB → PutTSDBBlock path; round-trips the block via tsdb.OpenBlock and verifies all 56 samples land.
  • TestTSDBBlockBuilder_WideTimestampSpanNoOOB — 5-minute span between earliest and latest samples in a single window builds cleanly.

Before/after evidence

Reproducer on baseline (unfixed) main:

=== RUN   TestIssue46Repro
    issue46_repro_test.go:50:
        Error: tsdb appender.Append: out of bounds
        Test:  TestIssue46Repro
        Messages: trial 0
--- FAIL: TestIssue46Repro (0.01s)

After fix:

=== RUN   TestTSDBBlockBuilder_RotatingCardinalityNoOOB
--- PASS: TestTSDBBlockBuilder_RotatingCardinalityNoOOB (2.90s)
=== RUN   TestFlushTSDB_OOBDoesNotCrashProcessor
--- PASS: TestFlushTSDB_OOBDoesNotCrashProcessor (0.06s)
PASS
ok  github.com/open-telemetry/opentelemetry-collector-contrib/processor/gorillas3processor   6.275s

Closes #46 (gorillas3 TSDB OOB blocker).

Test plan

  • Unit tests pass (full package)
  • Reproducer fails on baseline, passes after fix
  • Live MVP soak — deferred; worktree's submodule chain isn't populated, and a parallel agent is reducing fake-exporter cardinality. The unit-test reproducer captures the exact failure path.

🤖 Generated with Claude Code

… metrics

Root cause: Prometheus TSDB head appender locks `minValidTime` at
`firstAppendedSampleTs - chunkRange/2` for the appender's lifetime.
With Go's randomised map iteration, if the first-visited series in
a flush window held samples in e.g. the [40..55s] slice, the floor
became 10s and any subsequent series with samples at t<10s tripped
`storage.ErrOutOfBounds`. PR#338's fake-exporter `unique_users_per_min`
rotating user_id pool made this trip on every flush, crashing the
agent 60s after start.

Fix: flatten samples across all series, sort globally by timestamp
ascending, then drive Append. Anchors `minValidTime` at the smallest
ts in the window so every other sample lands above the floor. Also
tolerates `ErrOutOfBounds` defensively (skip + count via new
`gorillas3_tsdb_oob_samples_dropped_total` counter) so future
late-arriving samples can't crash the agent.

Coverage: TestTSDBBlockBuilder_RotatingCardinalityNoOOB (50 trials of
randomized map iteration with non-overlapping per-series time slices),
TestFlushTSDB_OOBDoesNotCrashProcessor (full processor flush path),
TestTSDBBlockBuilder_WideTimestampSpanNoOOB (5-minute span tolerance).
Closes #46.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit d63cf79 into main May 9, 2026
zzylol added a commit that referenced this pull request May 9, 2026
… (#46) (#355)

Root cause: PR #353 spawned six gorillas3 instances (one per
per-family pipeline under the 5-sketch routing topology), each with
its own in-memory windowState. With the static placeholder's 60 s
flush interval and PR #338's fake-exporter cardinality, the agent
buffered the full per-pipeline working set in RAM BEFORE the first
flush ticker fired — peak crossed the 1.5 GiB per-agent memory
limit, the agent was OOM-killed, and zero TSDB blocks ever reached
MinIO. PR #354's flat-slice OOB-tolerance approach compounded the
problem at flush time (O(N_samples) flat slice + interleaved Head
chunk creation across every series) but the agent never lived long
enough for that path to fire.

Result in /tmp/asap-mvp-rerun-bug34/asap/measurements/accuracy.csv:
1713 archive_miss / 0 archive_ok (was 343 archive_ok pre-#354).

Two-part fix:

1) opentelemetry-collector-contrib-patch/processor/gorillas3processor:
   replace PR #354's flat-sort-and-skip path with a series-visit-
   ordered append. Each series' points are still sorted ascending
   locally, but series are now visited in ascending order of each
   series' EARLIEST sample timestamp. That guarantees the very first
   `app.Append(...)` carries the global minimum, anchoring the
   appender's `minValidTime = globalMin - chunkRange/2` so every
   other in-window sample passes the OOB check — without ever
   allocating an O(N_samples) flat slice or interleaving Head
   series creation. Memory footprint at flush is bounded by
   `max(series points, Head per-series state)` instead of total
   sample count. PR #354's defensive ErrOutOfBounds tolerance
   (`gorillas3_tsdb_oob_samples_dropped_total` counter + warn log,
   no rollback) is retained for genuinely late-arriving samples.

2) deploy/configs/asap-otel-agent-b6-asap-single-sketch.yaml:
   reduce gorillas3.window_interval from 60 s → 5 s. With six
   per-pipeline windowStates instead of one, the per-instance
   in-flight buffer is the dominant memory pressure. 5 s caps the
   per-pipeline window peak at ~150 MiB, putting the steady-state
   aggregate inside the agent's 1.5 GiB budget. tsdb_block_duration
   stays at 60 s so the on-S3 layout still matches the Thanos
   store-gateway sync interval; flushes just roll smaller sub-blocks
   that thanos-compact will merge.

Verification (90 s soak diagnose, repeated):
  before: mc ls myminio/asap-gorilla-tsdb/ → empty
          curl -s http://localhost:19092/api/v1/labels →
            {"data":["__name__"]} (no metric labels surfaced)
  after:  mc ls myminio/asap-gorilla-tsdb/ → 27 blocks across all
          six pipelines (countsketch_path, countminsketch_path,
          ddsketch_path, hll_path, kll_path, raw_passthrough)
          curl -s http://localhost:19092/api/v1/label/__name__/values →
            ["endpoint_request_freq","http_freshness_probe_archive",
             "http_freshness_probe_raw","http_freshness_probe_warm",
             "http_requests_total","http_requests_total_latency_ms",
             "request_size_bytes","top_endpoint_qps",
             "unique_users_per_min"]
          all 5 sketched metrics + raw + 3 freshness probes.

Coverage:
  TestTSDBBlockBuilder_HighCardinalityMemoryBound — 200-series /
  60-sample window with non-overlapping per-series time slices, so
  the global minimum lives on a different series from the global
  maximum. Asserts 0 OOB drops + every sample lands in the block.
  Pre-this-PR's algorithm would still have passed correctness but
  allocated the O(N) flat slice; this test pins the visit-order
  invariant going forward.

Existing TSDB OOB regressions (RotatingCardinalityNoOOB,
WideTimestampSpanNoOOB, FlushTSDB_OOBDoesNotCrashProcessor) all
still pass with the new visit-order path — same OOB-tolerance
contract, different memory footprint.

Closes #46.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the fix/gorillas3-tsdb-out-of-bounds branch May 9, 2026 18:00
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.

MVP demo: test-first validation of ASAPCollector + ASAPQuery-backend

1 participant