Address review comments: the keep-gating of tombstones relies on keep's
answers staying stable for the duration of the Checkpoint call, which the
Head guarantees via chunkSnapshotMtx; and the checkpoint test now verifies
walExpiries entries are retained/cleared around the truncation mint.
Signed-off-by: Patryk Prus <p@trykpr.us>
Co-authored-by: Cursor <cursoragent@cursor.com>
The <oauth2> configuration reference lists an http_headers block, but the
OAuth2 type in prometheus/common has no corresponding field (it embeds
ProxyConfig, but not HTTPHeaders, unlike HTTPClientConfig). Because config
is unmarshaled strictly, setting http_headers under an oauth2 block fails
with "field http_headers not found in type config.plain", so the
documentation advertises an option that cannot be used.
Remove the http_headers block from the oauth2 reference. The proxy_*
options above it remain, as OAuth2 does embed ProxyConfig.
Fixes#19103
Signed-off-by: Akanksha Trehun <akankshatrehun@gmail.com>
Route every mutation of the headChunks/headChunkCount pair through two
unexported helpers, pushHeadChunk and setHeadChunks, so the invariant
that the counter equals the list length is maintained in one place
instead of at scattered call sites. No behavior change.
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Each line skipped would use more stack space. In normal usage this is
only one or two lines, but bugs or malicious targets could turn this
into a crash.
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Collapse the cache fingerprint into a comparable headChunkCacheKey so
the hit check, populate, and reset are single expressions and future
fingerprint fields cannot be forgotten in the comparison.
Bound the cache buffer: apply the headChunksBufMaxCap policy when
moving to a different series, pre-size replacement collections from
headChunkCount, and release the cache on Close. Route the OOO wrapper's
cache enablement and Close through the headChunkReader methods and
release its fallback collection buffer on Close, and propagate chunk
reader Close errors in the head-and-OOO queriers.
Zero the collection tail inside collectHeadChunks itself instead of at
every call site, and document the contract there.
Test the cache-hit path with a sentinel (a re-collection for the same
series rewrites the same backing array, so pointer identity of the
slice cannot distinguish a hit from a miss), extract a shared test-head
helper, and assert outside the series lock so a failing assertion
cannot deadlock the cleanup's Close.
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
A full-range tombstone (a single interval covering (MinInt64, MaxInt64),
written when a series is fully deleted from the head) is mishandled by
WAL replay and checkpointing in three ways, each able to cause unknown
series references and dropped samples on later replays:
1. Replay applies the tombstone by queueing the series deletion onto a
concurrent worker, but the series stays in the hash index until the
deletion applies. A later series record with the same labels (the
series recreated after deletion) is deduplicated onto the condemned
ref via multiRef, and the queued deletion then tears down the
recreated series together with its samples. Now the series is
synchronously removed from the hash index before the deletion is
queued, so recreations always create a fresh series.
2. Deletions applied during replay set no WAL expiry for the deleted
ref, so the next checkpoint dropped the series record while later
segments still held samples for it, and the following replay
discarded those samples. deleteSeriesByID now records a WAL expiry
at the series' max sample time.
3. lastSeriesID was only advanced from series records, so the ref of a
tombstoned series whose record had left the WAL could be reissued to
a new series, which a stranded tombstone would then delete on the
next replay. Replay now advances lastSeriesID past every tombstone
ref. Checkpoints also retained any tombstone with an interval
reaching past mint - always true for full-range tombstones - even
after dropping the series record; tombstones are now dropped when
their ref fails keep(), the same rule used for series records.
Signed-off-by: Patryk Prus <p@trykpr.us>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Patryk Prus <p@trykpr.us>
The head-chunk cache fingerprint (series ref, head pointer, mmapped
count) could not detect truncateChunksBefore removing older head
chunks: the head pointer survives and, for a series with no mmapped
chunks, the mmapped count stays 0, while firstChunkID advances. A
cache-enabled reader (range queries through the head-and-OOO reader)
then resolved chunk IDs against the stale slice and returned the wrong
chunk, or failed with ErrNotFound.
Include firstChunkID in the fingerprint; it advances on every
truncation and never regresses.
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
The doc comment stated the method returns the start timestamp "if exists
or nil", but StartTimestamp returns int64, which can never be nil. When the
start timestamp is unknown the method returns the sentinel 0, matching the
Parser.StartTimestamp interface contract and the sibling Protobuf and Prom
parser doc comments. Correct the wording to say 0 instead of nil.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Upgrade consul/api to v1.33.7, as well as serf to v0.10.4, to get rid of
vulnerable mapstructure dependency.
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
This will allow the simplification of the directIOWriter.
PR #7573 introduced a .tmp file + rename pattern in cutSegmentFile to
prevent mmap from seeing empty files after a crash. However, without an
fsync on the file itself, a crash could still leave the renamed file
with zero content or sth else. PR #8061 added repairLastChunkFile as the real fix,
extended in PR #11338 to detect zero magic numbers.
The .tmp indirection is redundant, both callers have crash safety:
- Head chunks: repairLastChunkFile removes corrupt last files on
startup. No intermediary broken file can exist (handleChunkWriteError
panics, so the corrupt file is always the last one).
- Compaction: files live in a .tmp-for-creation block directory, cleaned
up entirely on failure or restart.
Remove the pattern for fewer syscalls and a simpler write path.
Signed-off-by: Ayoub Mrini <ayoubmrini424@gmail.com>
These tests (and their _AppendV2 variants) waited at most one second for
CompactOOOHead to finish after closing the blocking querier. The
compaction goroutine only re-checks for pending readers in 500ms
intervals (WaitForPendingReadersForOOOChunksAtOrBefore) and still has to
garbage collect chunks and truncate the WBL after it notices the close,
so the one second window sits right at the failure boundary and
regularly times out on slow Windows CI runners.
Widen the wait to a minute (require.Eventually returns as soon as the
condition holds, so fast runners lose nothing) and move the require
calls out of the compaction goroutine: require must not be called from
non-test goroutines, and its late FailNow there produced the confusing
secondary "compact ooo head: populate block: context canceled" failures
when the DB was closed during cleanup after the timeout.
Part of #17941
Signed-off-by: junkerderprovinz <jdp@braethoria.com>
When a rule group is removed or renamed on reload, Manager.Update deletes
the rule_group-labeled series for most group metrics but missed
GroupLastRuleDurationSum and GroupLastRestoreDuration, which carry the
same rule_group label. Their series leaked on every reload that dropped a
group, growing memory and /metrics cardinality over time.
Delete both alongside the existing cleanup, and extend TestMetricsUpdate
to cover rule_group_last_rule_duration_sum_seconds so the leak is caught.
Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@gmail.com>
Convert TestNewTokenProvider from a sequential testify suite method
into a table-driven test where each case uses its own mock credential
and t.Parallel(). The four valid auth cases each sleep 4s waiting for
token refresh; overlapping them cuts package wall-clock time from ~16s
to ~4s.
Refs #15185
Signed-off-by: Sahitya Chandra <sahityajb@gmail.com>
Each test creates its own Manager and prometheus.Registry, so they share
no mutable state. Mark top-level tests and independent table-driven
subtests with t.Parallel() to cut package wall-clock time.
Refs #15185
Before: ~5.0s (go test ./discovery -count=1 -parallel 1)
After: ~0.74s (go test ./discovery -count=1)
Signed-off-by: Sahitya Chandra <sahityajb@gmail.com>
The doc comments on bucketLimitAppender and bucketLimitAppenderV2 were
copied from limitAppender and never updated: they claimed to limit the
number of total appended samples, but both types cap the number of
buckets in native histograms, reducing the histogram resolution or
returning errBucketLimit when the limit is exceeded.
The bucketLimitAppenderV2 comment additionally named the wrong type
(bucketLimitAppender), and the adjacent limitAppenderV2 comment named
limitAppender. Correct the identifiers so each doc comment starts with
the type it documents.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
* feat(promtool): Add flag --remote-write.path=/api/v1/write
* docs(promtool): Generate cli docs for new flag
Signed-off-by: Steffen Baarsgaard <steff.bpoulsen@gmail.com>
Co-authored-by: Varsha U N <varshaun58@gmail.com>
