Commit Graph

143 Commits (9aa6e041d3b6cc9b7041c93d1bf34b3b11d2294e)

Author SHA1 Message Date
Fiona Liao c599d37668
Always return unknown hint for first sample in non-gauge histogram chunk (#15343)
Always return unknown hint for first sample in non-gauge histogram chunk

---------

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
Co-authored-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-11-12 15:14:06 +01:00
Matthieu MOREL af1a19fc78 enable errorf rule from perfsprint linter
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2024-11-06 16:50:36 +01:00
Bryan Boreham e8c2d916ec lint
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-09-18 15:23:46 +01:00
Bryan Boreham 648a668835 [PERF] Chunk encoding: combine timestamp writes
Instead of a 2-bit write followed by a 14-bit write, do two 8-bit
writes, which goes much faster since it avoids looping.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-09-18 13:19:21 +01:00
Bryan Boreham b9a9689aae [PERF] Chunk encoding: simplify writeByte
Rather than append a zero then set the value at that position, append the value.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-09-18 13:19:04 +01:00
Bryan Boreham b65f1b6560 TSDB: Improve xor-chunk benchmarks
Benchmarks must do the same work N times.
Run 3 cases, where the values are constant, vary a bit, and vary a lot.

Also aim for 120 samples same as TSDB default.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-09-18 13:14:49 +01:00
Antoine Pultier d90d0976b5
fix(bstream/writeByte): ensure it appends only one byte (#14854)
fix(bstream/writeByte): ensure it appends only one byte

Signed-off-by: Antoine Pultier <antoine.pultier@sintef.no>
2024-09-17 16:28:33 +02:00
Nathan Baulch 50cd453c8f
chore: Fix typos (#14868)
* Fix typos

---------

Signed-off-by: Nathan Baulch <nathan.baulch@gmail.com>
2024-09-10 22:32:03 +02:00
Joshua Hesketh f2064c7987
NH: Do not re-use spans between histograms (#14771)
promql, tsdb (histograms): Do not re-use spans between histograms

When multiple points exist with the same native histogram schemas they
share their spans.
This causes a problem when a native histogram (NH) schema is modified (for example, during
a Sum) then the other NH's with the same spans are also modified. As such,
we should create a new Span for each NH. This will ensure NH's interfaces
are safe to use without considering the effect on other histograms.

At the moment this doesn't present itself as a problem because in all
aggregations and functions operating on native histograms they are copied
by the promql query engine first.

Signed-off-by: Joshua Hesketh <josh@nitrotech.org>

---------

Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
2024-09-04 12:07:16 +02:00
György Krajcsovits a693dd19f2 Fix: chunkenc.MockSeriesIterator
Starts its index from 0 , but users call Next() before first sample
so it needs to start from -1

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-08-30 16:44:36 +02:00
György Krajcsovits 98ecdf3589 Fix corrupting spans via iterator sharing
Iterator may share spans without copy, so we always have to make a copy
before modification - copy-on-write.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-08-06 16:51:20 +02:00
György Krajcsovits d2f6fa7289 Fix lint error
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-08-06 13:24:46 +02:00
György Krajcsovits 1b6d1366d8 Fix re-code histogram and chunk re-code conflict
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-08-06 13:09:17 +02:00
György Krajcsovits aff089a014 Reproduce recoding bug with new and missing buckets
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-08-06 10:51:44 +02:00
George Krajcsovits 00ab05c3b9
Native histograms: fix spurios counter reset when merging recoded chunk to normal chunk (#14513)
* chunkenc: allow missing empty buckets on histogram append

Allow appending to chunks when the histogram to be added is missing
some buckets, but the missing buckets are empty in the chunk.
For example bucket at index 5 is present in the chunk, but its value
is 0 and the new histogram doesn't have a bucket at index 5.

This fixes an issue of merging chunks where one chunk was recoded to
retroactively have some empty buckets in all the histograms and we are
merging in a histogram that doesn't have the empty bucket (because it
was not recoded yet).

The operation alters the histogram that is being added, however this has
already been the case when appending gauge histograms. Thus the test
TestHistogramSeriesToChunks in storage package is changed to explicitly
test what happened to the appended histogram - Compact(0) call is removed.

The new expandIntSpansAndBuckets and expandFloatSpansAndBuckets functions
are a merge of expandSpansForward and counterResetInAnyBucket and
counterResetInAnyFloatBucket.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-08-01 09:22:32 +02:00
Arve Knudsen e410a215fb Fix a couple of comments
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
2024-07-05 15:25:42 +02:00
Jeanette Tan 9adc1699c3 fix according to code review
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-06-07 18:50:59 +08:00
Jeanette Tan f028496133 Merge branch 'main' into nhcb
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-05-14 16:20:15 +08:00
Arve Knudsen d699dc3c77
Fix language in docs and comments (#14041)
Fix language in docs and comments

---------

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Co-authored-by: Björn Rabenstein <github@rabenste.in>
2024-05-08 17:57:09 +02:00
Arve Knudsen 108a6bc9f6 tsdb/chunkenc.Pool: Refactor Get and Put
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
2024-05-08 13:37:25 +02:00
György Krajcsovits bcafa5f1f9 Merge remote-tracking branch 'upstream/main' into update-nhcb 2024-04-24 11:06:59 +02:00
Matthieu MOREL 6f595c6762
golangci-lint: enable whitespace linter (#13905)
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2024-04-11 09:27:54 +01:00
George Krajcsovits 4eab18abd6
[nhcb branch] Use single bit to differentiate between optimized bounds and floats (#13828)
* Use single bit to differentiate between optimized bounds and floats

Use one bit to decide what kind of data to read/write.
This reduces storage need of floats from 72 bits to 65 bits and makes the
integers store in 5 to 32 bits instead of 16.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: George Krajcsovits <krajorama@users.noreply.github.com>
Co-authored-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-03-27 18:40:59 +01:00
zenador 4acbb7dea6
Add custom buckets to native histogram chunks encoding (#13706)
* add custom bounds to chunks encoding
* change custom buckets schema number
* rename custom bounds to custom values

Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-03-22 14:36:39 +01:00
Filip Petkovski 583f3e587c
Optimize histogram iterators (#13340)
Optimize histogram iterators

Histogram iterators allocate new objects in the AtHistogram and
AtFloatHistogram methods, which makes calculating rates over long
ranges expensive.

In #13215 we allowed an existing object to be reused
when converting an integer histogram to a float histogram. This commit follows
the same idea and allows injecting an existing object in the AtHistogram and
AtFloatHistogram methods. When the injected value is nil, iterators allocate
new histograms, otherwise they populate and return the injected object.

The commit also adds a CopyTo method to Histogram and FloatHistogram which
is used in the BufferedIterator to overwrite items in the ring instead of making
new copies.

Note that a specialized HPoint pool is needed for all of this to work 
(`matrixSelectorHPool`).

---------

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
2024-01-23 17:02:14 +01:00
Matthieu MOREL 8f6cf3aabb tsdb: use Go standard errors
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2023-12-11 12:18:54 +00:00
Filip Petkovski 10a82f87fd
Enable reusing memory when converting between histogram types
The 'ToFloat' method on integer histograms currently allocates new memory
each time it is called.

This commit adds an optional *FloatHistogram parameter that can be used
to reuse span and bucket slices. It is up to the caller to make sure the
input float histogram is not used anymore after the call.

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
2023-12-08 10:22:59 +01:00
Matthieu MOREL 9c4782f1cc
golangci-lint: enable testifylint linter (#13254)
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2023-12-07 11:35:01 +00:00
Fiona Liao b8bcaef14d
Fix histogram append errors (#13201)
* Fix histogram append errors

We should check counterReset condition rather than okToAppend because if
there's a counter reset, okToAppend is always set to false.

Signed-off-by: Fiona Liao <fiona.y.liao@gmail.com>
2023-11-29 11:39:12 +01:00
Fiona Liao 5bee0cfce2
Change `ChunkReader.Chunk()` to `ChunkOrIterable()`
The ChunkReader interface's Chunk() has been changed to ChunkOrIterable(). 

This is a precursor to OOO native histogram support - with OOO native histograms, the chunks.Meta passed to Chunk() can result in multiple chunks being returned rather than just a single chunk (e.g. if oooMergedChunk has a counter reset in the middle). 

To support this, ChunkOrIterable() requires either a single chunk or an iterable to be returned. If an iterable is returned, the caller has the responsibility of converting the samples from the iterable into possibly multiple chunks. The OOOHeadChunkReader now returns an iterable rather than a chunk to prepare for the native histograms case. Also as a beneficial side effect, oooMergedChunk and boundedChunk has been simplified as they only need to implement the Iterable interface now, not the full Chunk interface.

---------

Signed-off-by: Fiona Liao <fiona.y.liao@gmail.com>
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
2023-11-28 11:14:29 +01:00
Matthieu MOREL b60f9f801e tsdb/chunkenc: use Go standard errors package
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2023-11-08 08:38:57 +00:00
Oleksandr Redko fa90ca46e5 ci(lint): enable godot; append dot at the end of comments
Signed-off-by: Oleksandr Redko <Oleksandr_Redko@epam.com>
2023-10-31 19:53:38 +02:00
Oleksandr Redko 8e5f0387a2
ci(lint): enable nolintlint and remove redundant comments (#12926)
Signed-off-by: Oleksandr Redko <Oleksandr_Redko@epam.com>
2023-10-31 12:35:13 +01:00
György Krajcsovits b2fa4d910a Fix more counterResetInAnyBucket edgecases
Case a) empty span is at the beginning of the spans.
Case b) two consequtive empty spans with positive offsets.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2023-09-13 15:18:40 +02:00
George Krajcsovits b6f903b5f9
Fix handling of explicit counter reset header in histograms. (#12772)
* Fix handling of explicit counter reset header in histograms.

Explicit counter reset were being ignored.
Also there was no unit test coverage.
Add test case for the first sample in a chunk.
Add test case for non first sample in chunk.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

---------

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2023-09-01 23:39:15 +02:00
Justin Lei 8ef7dfdeeb
Add a chunk size limit in bytes (#12054)
Add a chunk size limit in bytes

This creates a hard cap for XOR chunks of 1024 bytes.

The limit for histogram chunk is also 1024 bytes, but it is a soft limit as a histogram has a dynamic size, and even a single one could be larger than 1024 bytes.

This also avoids cutting new histogram chunks if the existing chunk has fewer than 10 histograms yet. In that way, we are accepting "jumbo chunks" in order to have at least 10 histograms in a chunk, allowing compression to kick in.

Signed-off-by: Justin Lei <justin.lei@grafana.com>
2023-08-24 15:21:17 +02:00
Oleg Zaytsev c810e7cae3
Fix typo in Appender.AppendHistogram() arg name
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
2023-08-04 10:21:16 +02:00
George Krajcsovits 6cd2d1621f
Hide histogram chunk append and reset header internals (#12352)
tsdb: Hide histogram chunk append and reset header internals

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: George Krajcsovits <krajorama@users.noreply.github.com>
2023-07-26 15:08:16 +02:00
György Krajcsovits c6618729c9 Fix HistogramAppender.Appendable array out of bound error
The code did not handle spans with 0 length properly.

Spans with length zero are now skipped in the comparison.
Span index check not done against length-1, since length is a unit32,
thus subtracting 1 leads to 2^32, not -1.

Fixes and unit tests for both integer and float histograms added.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2023-05-14 17:38:52 +02:00
Matthieu MOREL bae9a21200
Merge branch 'main' into linter/nilerr
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2023-04-19 19:56:39 +02:00
beorn7 5b53aa1108 style: Replace `else if` cascades with `switch`
Wiser coders than myself have come to the conclusion that a `switch`
statement is almost always superior to a statement that includes any
`else if`.

The exceptions that I have found in our codebase are just these two:

* The `if else` is followed by an additional statement before the next
  condition (separated by a `;`).
* The whole thing is within a `for` loop and `break` statements are
  used. In this case, using `switch` would require tagging the `for`
  loop, which probably tips the balance.

Why are `switch` statements more readable?

For one, fewer curly braces. But more importantly, the conditions all
have the same alignment, so the whole thing follows the natural flow
of going down a list of conditions. With `else if`, in contrast, all
conditions but the first are "hidden" behind `} else if `, harder to
spot and (for no good reason) presented differently from the first
condition.

I'm sure the aforemention wise coders can list even more reasons.

In any case, I like it so much that I have found myself recommending
it in code reviews. I would like to make it a habit in our code base,
without making it a hard requirement that we would test on the CI. But
for that, there has to be a role model, so this commit eliminates all
`if else` occurrences, unless it is autogenerated code or fits one of
the exceptions above.

Signed-off-by: beorn7 <beorn@grafana.com>
2023-04-19 17:22:31 +02:00
beorn7 c3c7d44d84 lint: Adjust to the lint warnings raised by current versions of golint-ci
We haven't updated golint-ci in our CI yet, but this commit prepares
for that.

There are a lot of new warnings, and it is mostly because the "revive"
linter got updated. I agree with most of the new warnings, mostly
around not naming unused function parameters (although it is justified
in some cases for documentation purposes – while things like mocks are
a good example where not naming the parameter is clearer).

I'm pretty upset about the "empty block" warning to include `for`
loops. It's such a common pattern to do something in the head of the
`for` loop and then have an empty block. There is still an open issue
about this: https://github.com/mgechev/revive/issues/810 I have
disabled "revive" altogether in files where empty blocks are used
excessively, and I have made the effort to add individual
`// nolint:revive` where empty blocks are used just once or twice.
It's borderline noisy, though, but let's go with it for now.

I should mention that none of the "empty block" warnings for `for`
loop bodies were legitimate.

Signed-off-by: beorn7 <beorn@grafana.com>
2023-04-19 17:10:10 +02:00
Matthieu MOREL fb3eb21230 enable gocritic, unconvert and unused linters
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2023-04-13 19:20:22 +00:00
Justin Lei 83f43982c9 Add support for native histograms to concreteSeriesIterator
Signed-off-by: Justin Lei <justin.lei@grafana.com>
2023-04-06 09:54:15 -07:00
Abhijit Mukherjee 8f6d5dcd45
Fix: getting rid of EncOOOXOR chunk encoding (#12111)
Signed-off-by: mabhi <abhijit.mukherjee@infracloud.io>
2023-03-16 15:53:47 +05:30
beorn7 1cfc8f65a3 histograms: Return actually useful counter reset hints
This is a bit more conservative than we could be. As long as a chunk
isn't the first in a block, we can be pretty sure that the previous
chunk won't disappear. However, the incremental gain of returning
NotCounterReset in these cases is probably very small and might not be
worth the code complications.

Wwith this, we now also pay attention to an explicitly set counter
reset during ingestion. While the case doesn't show up in practice
yet, there could be scenarios where the metric source knows there was
a counter reset even if it might not be visible from the values in the
histogram. It is also useful for testing.

Signed-off-by: beorn7 <beorn@grafana.com>
2023-01-25 16:57:21 +01:00
beorn7 57c18420ab histograms: General readability tweaks
- Adjust doc comments to go1.19 style.
- Break down some overly long lines.
- Minor doc comment tweaks and fixes.
- Some renaming.

Some rationales for the last point:

I have renamed “interjections” into “inserts”, mostly because it is
shorter, and the word shows up a lot by now (and the concept is
cryptic enough to not obfuscate it even more with abbreviations).

I have also tried to find more descriptive naming for the “compare
spans” functions.

Signed-off-by: beorn7 <beorn@grafana.com>
2023-01-19 13:26:42 +01:00
Mingjie Shao 78d3c4e823 tsdb: Fixed typo in Histogram
Signed-off-by: Mingjie Shao <com.jerryshao@jerryshao.com>
2023-01-16 18:13:45 +08:00
beorn7 6dcd03dbf3 tsdb: Add integer gauge histogram support
This follows what #11783 has done for float gauge histograms.

Signed-off-by: beorn7 <beorn@grafana.com>
2023-01-11 13:28:43 +01:00
Ganesh Vernekar 57bcbf1888
Merge pull request #11783 from codesome/gauge-histogram
tsdb: Add gauge histogram support
2023-01-10 19:06:08 +05:30