Fixes https://github.com/prometheus/prometheus/issues/15177
The basic idea here is to divide the samples to be commited into (sub)
batches whenever we detect that the same series receives a sample of a
type different from the previous one. We then commit those batches one
after another, and we log them to the WAL one after another, so that
we hit both birds with the same stone. The cost of the stone is that
we have to track the sample type of each series in a map. Given the
amount of things we already track in the appender, I hope that it
won't make a dent. Note that this even addresses the NHCB special case
in the WAL.
This does a few other things that I could not resist to pick up on the
go:
- It adds more zeropool.Pools and uses the existing ones more
consistently. My understanding is that this was merely an oversight.
Maybe the additional pool usage will compensate for the increased
memory demand of the map.
- Create the synthetic zero sample for histograms a bit more
carefully. So far, we created a sample that always went into its own
chunk. Now we create a sample that is compatible enough with the
following sample to go into the same chunk. This changed the test
results quite a bit. But IMHO it makes much more sense now.
- Continuing past efforts, I changed more namings of `Samples` into
`Floats` to keep things consistent and less confusing. (Histogram
samples are also samples.) I still avoided changing names in other
packages.
- I added a few shortcuts `h := a.head`, saving many characters.
TODOs:
- Address @krajorama's TODOs about commit order and staleness handling.
Signed-off-by: beorn7 <beorn@grafana.com>
See
https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize
for details.
This ran into a few issues (arguably bugs in the modernize tool),
which I will fix in the next commit, so that we have transparency what
was done automatically.
Beyond those hiccups, I believe all the changes applied are
legitimate. Even where there might be no tangible direct gain, I would
argue it's still better to use the "modern" way to avoid micro
discussions in tiny style PRs later.
Signed-off-by: beorn7 <beorn@grafana.com>
Right now Prometheus only logs when these operations are completed.
It's a bit surprising to see suddenly a message saying "I was busy doing X for the past N minutes"
so let's add a message when the operation starts, so it's easier to understand what Prometheus was doing at any point in time
when reading logs.
Signed-off-by: Lukasz Mierzwa <l.mierzwa@gmail.com>
Improve readability of "WAL segment loaded" by logging the duration
of each load. This helps make it easier to spot slow WAL file load
times.
Signed-off-by: SuperQ <superq@gmail.com>
* Remove experimental out-of-order native histogram flag
This feature has been available in Prometheus since September 2024,
and has no known issues. Therefore proposing to remove the flag
entirely and always have it on. Note that there are still two
settings that need to be configured (out-of-order time window > 0
and native histograms enabled) for this feature to work.
Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
* Update CHANGELOG
Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
* Keep feature flag with warning
Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
* Update CHANGELOG
Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
* Update tsdb/head_append.go
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
Signed-off-by: Fiona Liao <fiona.y.liao@gmail.com>
* Update CHANGELOG.md
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
Signed-off-by: Fiona Liao <fiona.y.liao@gmail.com>
* Update tsdb/head_append.go
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
Signed-off-by: Fiona Liao <fiona.y.liao@gmail.com>
* Additional cleanup of comments and test names
Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
---------
Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
Signed-off-by: Fiona Liao <fiona.y.liao@gmail.com>
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
Renames the head's deleted map to walExpiries, and creates entries for any
duplicate series records encountered during WAL replay, with the expiry set
to the highest current WAL segment number. Any subsequent WAL
checkpoints will see the duplicate series entry in the walExpiries map, and
keep the series record until the last WAL segment that could contain its
samples is deleted.
Other considerations:
WBL: series records aren't written to the WBL, so there are no duplicates to deal with
agent mode: has its own WAL replay logic that handles duplicate series records differently, and is outside the scope of this PR
* [ENHANCEMENT] TSDB: Improve calculation of space used by labels
The labels for each series in the Head take up some some space in the
Postings index, but far more space in the `memSeries` structure.
Instead of having the Postings index calculate this overhead, which is
a layering violation, have the caller pass in a function to do it.
Provide three implementations of this function for the three Labels
versions.
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
* Remove unused option from HeadOptions
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* Improve docs for appendable() method in head appender
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* Ingest CT (float) samples in Agent DB
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* allow for ingestion of CT native histogram
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* adding some verification for ct ts
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* Validating CT histogram before append and add newly created series to pending series
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* checking the wal for written samples
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* Checking for samples in test
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* adding case for validations
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* fixing comparison when dedupelabels is enabled
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* unite tests, use table testing
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* Implement CT related methods in timestampTracker for write storage
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* adding error case to test
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* removing unused fields
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* Updating lastTs for series when adding CT to invalidate duplicates
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
* making sure that updating the lastTS wont cause OOO later on in Commit();
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
---------
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
memChunk is a linked list, speed up some common operations when there's no need to iterate all elements on the list.
Signed-off-by: Łukasz Mierzwa <l.mierzwa@gmail.com>
For: #14355
This commit updates Prometheus to adopt stdlib's log/slog package in
favor of go-kit/log. As part of converting to use slog, several other
related changes are required to get prometheus working, including:
- removed unused logging util func `RateLimit()`
- forward ported the util/logging/Deduper logging by implementing a small custom slog.Handler that does the deduping before chaining log calls to the underlying real slog.Logger
- move some of the json file logging functionality to use prom/common package functionality
- refactored some of the new json file logging for scraping
- changes to promql.QueryLogger interface to swap out logging methods for relevant slog sugar wrappers
- updated lots of tests that used/replicated custom logging functionality, attempting to keep the logical goal of the tests consistent after the transition
- added a healthy amount of `if logger == nil { $makeLogger }` type conditional checks amongst various functions where none were provided -- old code that used the go-kit/log.Logger interface had several places where there were nil references when trying to use functions like `With()` to add keyvals on the new *slog.Logger type
Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>
The specialized version of sample add to the ring:
func addH(s hSample, buf []hSample, r *sampleRing) []hSample
func addFH(s fhSample, buf []fhSample, r *sampleRing) []fhSample
already correctly copy histogram samples from the reused hReader, fhReader
buffers, but the generic version does not. This means that the
data is overwritten on the next read if the sample ring has seen histogram
and float samples at the same time and switched to generic mode.
The `genericAdd` function (which was commented anyway) is by now quite
different from the specialized functions so that this commit deletes
it.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Check if headQuerier is nil before trying to use it.
* TestQueryOOOHeadDuringTruncate: unit test to check query during truncate
Regression test for #14822
* Simulate race between query and Compact()
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
We don't use seriesShard during DB initialization, so we can use the
same 8 bytes to store mmMaxTime, and save those during the rest of the
lifetime of the database.
This doesn't affect CPU performance.
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
In each case the following member requires 8-byte alignment, so moving
one beside the other shrinks memSeries from 176 to 168 bytes, when
compiled with `-tags stringlabels`.
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
* TSDB: Document what needs locking in memSeries
* TSDB: Lock around access to series labels
So we can modify them to reset the symbol-table.
* TSDB: Make label locking conditional on build tag
---------
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
* Pass affected labels to MemPostings.Delete
As suggested by @bboreham, we can track the labels of the deleted series
and avoid iterating through all the label/value combinations.
This looks much faster on the MemPostings.Delete call. We don't have a
benchmark on stripeSeries.gc() where we'll pay the price of iterating
the labels of each one of the deleted series.
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
* TSDB: Don't compact the head block when empty
Don't compact the Head block if there have not yet been any samples
appended.
Previously, the logic for determining if the head should be compacted
relied on the default values for min and max time and integer overflow
when they were checked in `Head.compactable()`. The check in
`Head.compactable()` effectively did `math.MinInt64 - math.MaxInt64`
which overflowed and wrapped to `1`. Since `1` is less than `1.5`
times the chunk range, compaction did not happen. This was the correct
behavior but relying on overflow wrapping is surprising.
This change add a method for checking if the min and max time for the
head is unset and uses it to short-circuit compaction in that case.
It also replaces several explicit checks for the default value to
determine if the head has not yet had any samples added.
Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
Dogfood native histograms.
Allow dependent projects to migrate to native histograms.
I took the defaults from client_golang.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
This PR is a reference implementation of the proposal described in #10420.
In addition to what described in #10420, in this PR I've introduced labels.StableHash(). The idea is to offer an hashing function which doesn't change over time, and that's used by query sharding in order to get a stable behaviour over time. The implementation of labels.StableHash() is the hashing function used by Prometheus before stringlabels, and what's used by Grafana Mimir for query sharding (because built before stringlabels was a thing).
Follow up work
As mentioned in #10420, if this PR is accepted I'm also open to upload another foundamental piece used by Grafana Mimir query sharding to accelerate the query execution: an optional, configurable and fast in-memory cache for the series hashes.
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Mutex is 8 bytes; RWMutex is 24 bytes and much more complicated. Since
`RLock` is only used in two places, `UpdateMetadata` and `Delete`,
neither of which are hotspots, we should use the cheaper one.
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Lifting an optimisation from Agent code, `seriesHashmap.del` can use
the unique series reference, doesn't need to check Labels.
Also streamline the logic for deleting from `unique` and `conflicts` maps,
and add some comments to help the next person.
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
