In-Storage & WAL Coupling - #4
Conversation
Implement compound WAL records and full pipeline coupling between samples and exemplars for issue prometheus#17857 (Option A): - Define compound record types (RefSampleV2, RefHistogramSampleV2, RefFloatHistogramSampleV2, RefCustomBucketsHistogramSampleV2) in tsdb/record. - Implement zero-allocation exemplar skipping decoders for replay efficiency. - Update HeadAppenderV2 to commit compound WAL records while dual-writing to in-memory ExemplarStorage to preserve PromQL query invariants. - Update WAL Watcher to stream compound records directly to QueueManager, populating PRW 2.0 TimeSeries with attached exemplars and PRW 1.0 conversions. - Add comprehensive test coverage verifying mixed WAL replay, rollback semantics, PRW 2.0/1.0 exemplar attachment, and zero-allocation decoding.
…fer pooling, and decoder header fix
dashpole
changed the title
|
|
||
| numEx := dec.Uvarint() | ||
| if numEx > 0 { | ||
| skipExemplars(dec, numEx) |
There was a problem hiding this comment.
This appends a numEx uvarint to record types 11/12/13 without changing the type number, so the same byte now means two different layouts. I encoded 4 samples with main's samplesV2 and decoded them here, and vice versa: both directions misparse. Usually that surfaces as invalid size → CorruptionErr → wal.Repair truncating the WAL, which is already data loss; in a minority of byte alignments it returns fewer samples with wrong refs/timestamps and err == nil. Please allocate a new record type for the compound layout instead of redefining 11/12/13.
|
|
||
| numEx := dec.Uvarint() | ||
| if numEx > 0 { | ||
| s.Exemplars = make([]RefExemplar, 0, numEx) |
There was a problem hiding this comment.
numEx comes straight off disk and is passed to make unbounded. Flipping this record's last byte to a 5-byte uvarint (~2³⁵) produces fatal error: runtime: out of memory — a runtime throw, so CorruptionErr/wal.Repair never gets a chance and Prometheus crash-loops on startup. The stripping path (dec.Samples) took 61.7s to return invalid size on the same input. Please bound numEx against dec.Len() before allocating and check dec.Err() inside skipExemplars's loop. (DecodeHistogram has the same shape on main, so a shared bound helper would be worth it.)
| // to end up with the correct order. | ||
| if len(b.floats) > 0 { | ||
| if len(b.floatsV2) > 0 { | ||
| if a.storeST || hasExemplarsFloats(b.floatsV2) { |
There was a problem hiding this comment.
tsdb/wlog/checkpoint.go:210 still round-trips these records through dec.Samples/enc.Samples, and RefSample has no exemplar field — so every checkpoint strips the coupled exemplars. Since head_append.go:1268 now suppresses the standalone Exemplars record whenever a V2 batch exists, the coupled copy is the only one, and checkpoint.go:343's preservation of record.Exemplars no longer helps. Checkpointing needs a compound branch, plus a test.
| s = a.bestEffortAppendSTZeroSample(s, ls, st, t, h, fh) | ||
| } | ||
|
|
||
| var attachedExemplars []record.RefExemplar |
There was a problem hiding this comment.
attachedExemplars is built from opts.Exemplars with no validation, but the in-memory write at line 220 still goes through ValidateExemplar. So duplicates (which validateExemplar explicitly calls "expected"), out-of-order exemplars and over-length label sets now reach the WAL and remote write while being dropped in memory — query_exemplars gives different answers before and after a replay of the same WAL. Validate first, then attach the survivors.
| // to end up with the correct order. | ||
| if len(b.floats) > 0 { | ||
| if len(b.floatsV2) > 0 { | ||
| if a.storeST || hasExemplarsFloats(b.floatsV2) { |
There was a problem hiding this comment.
Because AppenderV2 is the default scrape path here and scrape_append_v2.go fills AOptions.Exemplars unconditionally, this condition puts compound records into the WAL of a default-configured Prometheus, where main only ever wrote type 11/12/13 under --enable-feature=st-storage. That makes F2 and F3 reachable with no opt-in, and it writes exemplar bytes that replay discards when exemplar-storage is off (head_wal.go:177) while remote write still sends them. This needs a feature gate.
| Offset: r.Offset(), | ||
| rec := r.Record() | ||
| if h.opts.EnableExemplarStorage && dec.Type(rec) == record.SamplesV2 { | ||
| samplesV2, err := dec.SamplesV2(rec, nil) |
There was a problem hiding this comment.
dec.SamplesV2(rec, nil) allocates a fresh slice per WAL record, bypasses wlReplaySamplesPool, and then copies every sample into a second pooled slice — and the decoder does a make([]RefExemplar, 0, numEx) per sample. The PR's own BenchmarkRecord/DecodeRefSamplesV2 measures 2000 allocs/op and 96 kB/op for 1000 samples. Worth pooling the V2 slice and carving exemplars out of one per-record backing array before this lands.
| return true | ||
| } | ||
|
|
||
| func (t *QueueManager) AppendSamplesV2(samples []record.RefSampleV2) bool { |
There was a problem hiding this comment.
With send_exemplars on, everything now goes through AppendSamplesV2 and AppendExemplars is never called — so prometheus_remote_storage_exemplars_dropped_total stops incrementing entirely, and dataDropped.incr(1) under-counts a coupled unit. Worth incrementing the exemplar counters by len(s.Exemplars) on the drop paths here.
1 participant
Why is this change necessary?
Possible fix for prometheus#17857
In Remote Write 2.0 (PRW 2.0), the specification mandates:
Historically, Prometheus has treated exemplars as a series-level attribute rather than a sample-level attribute.
headAppenderV2.Commit()writes float/histogram samples and exemplars to separate WAL records (record.Samplesvs.record.Exemplars). When the WAL Watcher tails the WAL and pushes them into Remote Write shard queues, they are processed as independent series items. For PRW 2.0, this causes standalone exemplar records to be emitted aswritev2.TimeSeriescontaining exemplars with 0 samples and 0 histograms, directly violating the PRW 2.0 specification and causing failures in PRW 2.0 receivers (e.g. OpenTelemetry Collector, Mimir, Google Cloud Managed Prometheus).What does this PR do?
This PR implements Option A: End-to-End Per-Sample Association (In-Storage & WAL Coupling):
Compound WAL Records (
tsdb/record):RefSampleV2,RefHistogramSampleV2,RefFloatHistogramSampleV2, andRefCustomBucketsHistogramSampleV2.record.SamplesV2 (10),record.HistogramSamplesV2 (11),record.FloatHistogramSamplesV2 (12), andrecord.CustomBucketsHistogramSamplesV2 (13).skipExemplars) used during recovery/fast replay when exemplars are not needed.Head Appender & TSDB Dual-Write (
tsdb/head_append_v2.go,tsdb/head_wal.go):headAppenderV2.Appendto package attached exemplars into compound batches and commit compound WAL records./api/v1/query_exemplarsvia dual-write to in-memoryh.exemplars(CircularExemplarStorage).ExemplarStorageduring Head WAL replay recovery.WAL Watcher & Remote Write Pipeline (
tsdb/wlog/watcher.go,storage/remote/queue_manager.go):QueueManager.sendExemplars=falseor during non-tailing replay.populateV2TimeSeriesmaps attached exemplars directly to the metric'swritev2.TimeSeriesobject, guaranteeing 0 empty/standalone exemplar series.prompb.WriteRequest).Verification & Testing
go test -v -race ./tsdb/record/... ./tsdb/... ./tsdb/wlog/... ./storage/remote/...(All PASS)BenchmarkRecord/DecodeSamplesV2_ZeroAllocStripping: 0 allocs/opExemplarQuerierassertions after replay.