Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[changelog] Add a breaking warning for #6107 #1

Closed
wants to merge 240 commits into from

Conversation

verejoel
Copy link
Owner

Changes

Verification

Running receiver v0.32.0 with a mounted PVC /data directory, discovered the following logs:

ts=2023-08-23T19:38:33.719068277Z caller=main.go:135 level=error err="remove /data/tsdb/unknown/lock: permission denied\nremove storage lock files\nmain.startTSDBAndUpload\n\t/app/cmd/thanos/receive.go:601\nmain.runReceive\n\t/app/cmd/thanos/receive.go:279\nmain.registerReceive.func1\n\t/app/cmd/thanos/receive.go:99\nmain.main\n\t/app/cmd/thanos/main.go:133\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:250\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1598\npreparing receive command failed\nmain.main\n\t/app/cmd/thanos/main.go:135\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:250\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1598"

Adding the following pod security context resolves the issue:

securityContext:
  fsGroup: 1001

wallee94 and others added 30 commits April 19, 2023 17:18
Signed-off-by: Walther Lee <walthere.lee@gmail.com>
Signed-off-by: Walther Lee <walthere.lee@gmail.com>
* Remove custom Between test matcher

The upstream PR to efficientgo/e2e has been merged, so we can use  it from there.

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

* Run go mod tidy

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>

---------

Signed-off-by: Douglas Camata <159076+douglascamata@users.noreply.github.com>
Signed-off-by: Walther Lee <walthere.lee@gmail.com>
Signed-off-by: Walther Lee <walthere.lee@gmail.com>
Signed-off-by: Walther Lee <walthere.lee@gmail.com>
* Implemented native histogram support for qfe and query UI

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Fixed marshalling for histograms in qfe

Started working on native histogram query ui

Copied histogram implementation for graph

Added query range support for native histograms in qfe

Use prom model (un-)marshal for native histograms in qfe

Use prom model (un-)marshal for native histograms in qfe

Fixed sample and sample stream marshal fn

Extended qfe native histogram e2e tests

Added copyright to qfe queryrange compat

Added query range test fo histograms and try to fix ui tests

Fixed DataTable test

Review feedback

Fixed native histogram e2e test

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Add histogram support for ApplyCounterResetsSeriesIterator

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Made assets

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Add chnagelog

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Fixed changelog

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Fixed qfe

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Fixed PrometheusResponse minTime for histograms in qfe

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Updated prometheus common to v0.40.0 and queryrange.Sample fixes

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Updated Readme

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Addressed PR comments

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

trigger tests

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Made assets

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* Made assets

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* fixed tsdbutil references

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* fixed imports

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* Enabled pushdown for query native hist test and removed ToDo

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* Refactored native histogram query UI

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

---------

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
* store: add streamed snappy encoding for postings list

We've noticed that decoding Snappy compressed postings list
takes a lot of RAM:

```
(pprof) top
Showing nodes accounting for 1427.30GB, 67.55% of 2112.82GB total
Dropped 1069 nodes (cum <= 10.56GB)
Showing top 10 nodes out of 82
      flat  flat%   sum%        cum   cum%
         0     0%     0%  1905.67GB 90.20%  golang.org/x/sync/errgroup.(*Group).Go.func1
    2.08GB 0.098% 0.098%  1456.94GB 68.96%  github.com/thanos-io/thanos/pkg/store.(*blockSeriesClient).ExpandPostings
    1.64GB 0.078%  0.18%  1454.87GB 68.86%  github.com/thanos-io/thanos/pkg/store.(*bucketIndexReader).ExpandedPostings
    2.31GB  0.11%  0.29%  1258.15GB 59.55%  github.com/thanos-io/thanos/pkg/store.(*bucketIndexReader).fetchPostings
    1.48GB  0.07%  0.36%  1219.67GB 57.73%  github.com/thanos-io/thanos/pkg/store.diffVarintSnappyDecode
 1215.21GB 57.52% 57.87%  1215.21GB 57.52%  github.com/klauspost/compress/s2.Decode
```

This is because we are creating a new []byte slice for the decoded data
each time. To avoid this RAM usage problem, let's stream the decoding
from a given buffer. Since Snappy block format doesn't support streamed
decoding, let's switch to Snappy stream format which is made for exactly
that.

Notice that our current `index.Postings` list does not
support going back through Seek() even if theoretically one could want
something like that. Fortunately, to search for posting intersection, we
need to only go forward.

Benchmark data:

```
name                                                          time/op
PostingsEncodingDecoding/10000/raw/encode-16                  71.6µs ± 3%
PostingsEncodingDecoding/10000/raw/decode-16                  76.3ns ± 4%
PostingsEncodingDecoding/10000#01/snappy/encode-16            73.3µs ± 1%
PostingsEncodingDecoding/10000#01/snappy/decode-16            1.63µs ± 6%
PostingsEncodingDecoding/10000#02/snappyStreamed/encode-16     111µs ± 2%
PostingsEncodingDecoding/10000#02/snappyStreamed/decode-16    14.5µs ± 7%
PostingsEncodingDecoding/100000/snappyStreamed/encode-16      1.09ms ± 2%
PostingsEncodingDecoding/100000/snappyStreamed/decode-16      14.4µs ± 4%
PostingsEncodingDecoding/100000#01/raw/encode-16               710µs ± 1%
PostingsEncodingDecoding/100000#01/raw/decode-16              79.3ns ±13%
PostingsEncodingDecoding/100000#02/snappy/encode-16            719µs ± 1%
PostingsEncodingDecoding/100000#02/snappy/decode-16           13.5µs ± 4%
PostingsEncodingDecoding/1000000/raw/encode-16                7.14ms ± 1%
PostingsEncodingDecoding/1000000/raw/decode-16                81.7ns ± 9%
PostingsEncodingDecoding/1000000#01/snappy/encode-16          7.52ms ± 3%
PostingsEncodingDecoding/1000000#01/snappy/decode-16           139µs ± 4%
PostingsEncodingDecoding/1000000#02/snappyStreamed/encode-16  11.4ms ± 4%
PostingsEncodingDecoding/1000000#02/snappyStreamed/decode-16  15.5µs ± 4%

name                                                          alloc/op
PostingsEncodingDecoding/10000/raw/encode-16                  13.6kB ± 0%
PostingsEncodingDecoding/10000/raw/decode-16                   96.0B ± 0%
PostingsEncodingDecoding/10000#01/snappy/encode-16            25.9kB ± 0%
PostingsEncodingDecoding/10000#01/snappy/decode-16            11.0kB ± 0%
PostingsEncodingDecoding/10000#02/snappyStreamed/encode-16    16.6kB ± 0%
PostingsEncodingDecoding/10000#02/snappyStreamed/decode-16     148kB ± 0%
PostingsEncodingDecoding/100000/snappyStreamed/encode-16       148kB ± 0%
PostingsEncodingDecoding/100000/snappyStreamed/decode-16       148kB ± 0%
PostingsEncodingDecoding/100000#01/raw/encode-16               131kB ± 0%
PostingsEncodingDecoding/100000#01/raw/decode-16               96.0B ± 0%
PostingsEncodingDecoding/100000#02/snappy/encode-16            254kB ± 0%
PostingsEncodingDecoding/100000#02/snappy/decode-16            107kB ± 0%
PostingsEncodingDecoding/1000000/raw/encode-16                1.25MB ± 0%
PostingsEncodingDecoding/1000000/raw/decode-16                 96.0B ± 0%
PostingsEncodingDecoding/1000000#01/snappy/encode-16          2.48MB ± 0%
PostingsEncodingDecoding/1000000#01/snappy/decode-16          1.05MB ± 0%
PostingsEncodingDecoding/1000000#02/snappyStreamed/encode-16  1.47MB ± 0%
PostingsEncodingDecoding/1000000#02/snappyStreamed/decode-16   148kB ± 0%

name                                                          allocs/op
PostingsEncodingDecoding/10000/raw/encode-16                    2.00 ± 0%
PostingsEncodingDecoding/10000/raw/decode-16                    2.00 ± 0%
PostingsEncodingDecoding/10000#01/snappy/encode-16              3.00 ± 0%
PostingsEncodingDecoding/10000#01/snappy/decode-16              4.00 ± 0%
PostingsEncodingDecoding/10000#02/snappyStreamed/encode-16      4.00 ± 0%
PostingsEncodingDecoding/10000#02/snappyStreamed/decode-16      5.00 ± 0%
PostingsEncodingDecoding/100000/snappyStreamed/encode-16        4.00 ± 0%
PostingsEncodingDecoding/100000/snappyStreamed/decode-16        5.00 ± 0%
PostingsEncodingDecoding/100000#01/raw/encode-16                2.00 ± 0%
PostingsEncodingDecoding/100000#01/raw/decode-16                2.00 ± 0%
PostingsEncodingDecoding/100000#02/snappy/encode-16             3.00 ± 0%
PostingsEncodingDecoding/100000#02/snappy/decode-16             4.00 ± 0%
PostingsEncodingDecoding/1000000/raw/encode-16                  2.00 ± 0%
PostingsEncodingDecoding/1000000/raw/decode-16                  2.00 ± 0%
PostingsEncodingDecoding/1000000#01/snappy/encode-16            3.00 ± 0%
PostingsEncodingDecoding/1000000#01/snappy/decode-16            4.00 ± 0%
PostingsEncodingDecoding/1000000#02/snappyStreamed/encode-16    4.00 ± 0%
PostingsEncodingDecoding/1000000#02/snappyStreamed/decode-16    5.00 ± 0%
```

Compression ratios are still the same like previously:

```
$ /bin/go test -v -timeout 10m -run ^TestDiffVarintCodec$ github.com/thanos-io/thanos/pkg/store
[snip]
=== RUN   TestDiffVarintCodec/snappy/i!~"2.*"
    postings_codec_test.go:73: postings entries: 944450
    postings_codec_test.go:74: original size (4*entries): 3777800 bytes
    postings_codec_test.go:80: encoded size 44498 bytes
    postings_codec_test.go:81: ratio: 0.012
=== RUN   TestDiffVarintCodec/snappyStreamed/i!~"2.*"
    postings_codec_test.go:73: postings entries: 944450
    postings_codec_test.go:74: original size (4*entries): 3777800 bytes
    postings_codec_test.go:80: encoded size 44670 bytes
    postings_codec_test.go:81: ratio: 0.012
```

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* store: clean up postings code

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* store: fix estimation

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* store: use buffer.Bytes()

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* store/postings_codec: reuse extgrpc compressors/decompressors

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: add item

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: clean up whitespace

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

---------

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
With lots of blocks it could take some time to fill this no compact
marked map hence replace it atomically. I believe this leads to problems
in the compaction planner where it picks up no compact marked blocks
because meta syncer does synchronizations concurrently.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
…-func

Reuse buffers for label comparison
Signed-off-by: Walther Lee <walthere.lee@gmail.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Philip Gough <philip.p.gough@gmail.com>
Signed-off-by: Walther Lee <walthere.lee@gmail.com>
* Replaced summary in extprom metrics with histogram

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* Added changelog

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* Removed unused parameters from NewInstrumentationMiddleware

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

* Reverted NewInstrumentationMiddleware

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

---------

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
This commit exposes the label set alongside the min and max time
for each TSDB covered by a Store.

This information is used to scope the min time for a remote query
so that we do not produce partial aggregates in distriuted mode.

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>

Cleanup

Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
Signed-off-by: Sebastian Rabenhorst <sebastian.rabenhorst@shopify.com>
…-heap-less-func

Ignore common label-values in ProxyResponseHeap sort function
* store: fix inconsistent error for series limits

Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com>

* update changelog

Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com>

* Update pkg/store/bucket.go

Co-authored-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com>

* Update pkg/store/bucket.go

Co-authored-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com>

* rename labelValues serires liimiter test function

Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com>

---------

Signed-off-by: Thibault Mange <22740367+thibaultmg@users.noreply.github.com>
Co-authored-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Switch from nytimes gzip library to the klaustpost's gzip code. The old
gzip HTTP handler shows up a lot in allocs so that's how I ended up
doing this change.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
* Issue#5947 OTEL_TRACES_SAMPLER env var

Signed-off-by: shayyxi <shazi12384@gmail.com>

* Test correction

Signed-off-by: shayyxi <shazi12384@gmail.com>

* doc failure correction. parse float argument correction.

Signed-off-by: shayyxi <shazi12384@gmail.com>

* added the changelog.

Signed-off-by: shayyxi <shazi12384@gmail.com>

* ran make docs to fix the build failure.

Signed-off-by: shayyxi <shazi12384@gmail.com>

* corrected the incorrect change in tools.md

Signed-off-by: shayyxi <shazi12384@gmail.com>

* fixed review comments.

Signed-off-by: shayyxi <shazi12384@gmail.com>

---------

Signed-off-by: shayyxi <shazi12384@gmail.com>
Signed-off-by: Shazi <42436533+shayyxi@users.noreply.github.com>
Co-authored-by: shayyxi <shazi12384@gmail.com>
Use storepb.SeriesServer instead of the concrete struct. This allows
implementing functionality on top of the proxy.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
…e shrinking a redis cluster. redis/rueidis#209 (thanos-io#6342)

* use github.com/onsi/gomega/gleak to detect goroutine leak with timeout

Signed-off-by: Rueian <rueiancsie@gmail.com>
philipgough and others added 29 commits August 1, 2023 16:51
…ted (thanos-io#6576)

Signed-off-by: Philip Gough <philip.p.gough@gmail.com>
Upgrade objstore to include the dependency bump of minio-io. This should
fix: thanos-io#6541

Signed-off-by: Jacob Baungard Hansen <jacobbaungard@redhat.com>
Signed-off-by: Ben Ye <benye@amazon.com>
* store/bucket: remove sort.Slice data race

The matchers slice is now sorted in each call but that introduces a data
race because the slice is shared between all calls. Do the sorting once
on the outermost function.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* store: add test for ExpandPostings() race

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

---------

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
We need to lock r.mtx around stats manipulation.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Fix the test by using proto.Equal() instead of using the not
deterministic String(). See
https://github.com/protocolbuffers/protobuf-go/blob/master/internal/encoding/text/encode.go#L229.

Fixes thanos-io#6590

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
* go.mod: update Redis client

Update client to fix redis/rueidis#322.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* *: run go mod tidy

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

---------

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
* add metrics to track number of chunk refetches

Signed-off-by: Ben Ye <benye@amazon.com>

* update changelog

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
The same aggregator is shared between queries which doesn't make sense
as each aggregator should only aggregate Select stats available to it.
Fixes a race and query metrics.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
…6592)

* fix vertical sharding bug in without and union operations

Signed-off-by: Ben Ye <benye@amazon.com>

* changelog

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>
* query/endpointset: fix races

- Consistently hold mutex while accessing er.metadata
- Capture range loop variable

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* query/endpointset: fix getStatus() race

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

---------

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
…nternal-dedup

Resort store response set on internal label dedup
Snappy works on byte level and it can cut two different chunks in the
middle of a varint. Thus, if there's some error from the Decbuf then
fill up the buffer and try reading a varint again. Added repro test.

Closes thanos-io#6545.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Document the why/what from
thanos-io#6257 in the Querier
documentation.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
From the Go docs:

  "If the map is nil, the number of iterations is 0." [1]

Therefore, an additional nil check for before the loop is unnecessary.

[1]: https://go.dev/ref/spec#For_range

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Trying to build on go1.21 fails with a scary error about the GC moving
heap objects. This is due to a conservative assumption around future go
releases in go4.org/unsafe/assume-no-moving-gc

A more recent version actually checks with the go runtime, see:

https://go-review.googlesource.com/c/go/+/498121

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>
Signed-off-by: GitHub <noreply@github.com>
Co-authored-by: fpetkovski <fpetkovski@users.noreply.github.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Chunk reader needs to wait until the chunk loading ends in Close()
because otherwise there will be a race between appending to r.chunkBytes
and reading from it.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
…abel (thanos-io#6605)

* vertically shard binary expression even if no matching labels

Signed-off-by: Ben Ye <benye@amazon.com>

* add changelog

Signed-off-by: Ben Ye <benye@amazon.com>

* fix test

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>
…#6612)

In the case that we have pushed down queries and internal labels that
are overriden by external labels we are not flushing the sorted response.

Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
The remote engine implementation currently converts warnings to errors.
This prevents using partial response with the distributed engine since every
warning will cause a query to fail.

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
Signed-off-by: Vanshika <vanshikav928@gmail.com>
The marked change requires that users set a security context so that mounted volumes (PVCs in particular) will be writable by the `thanos` user.

Signed-off-by: Joel Verezhak <46783161+verejoel@users.noreply.github.com>
@verejoel verejoel closed this Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet