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.

Sign up for GitHub

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

Jump to bottom

Export metadata fetcher metrics #3660

Merged
merged 1 commit into from
Feb 26, 2021
Merged

Export metadata fetcher metrics #3660

merged 1 commit into from
Feb 26, 2021

Conversation

pracucci
Copy link
Contributor

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

In Cortex we're experimenting with a per-tenant bucket index which is a single json indexing blocks and deletion marks, so that queriers and store-gateway only need to periodically fetch the bucket index to discover new/deleted blocks instead of running "list objects" operations. If turns out to work well, it's a pattern we may consider backporting to Thanos too.

To implement it in the Cortex store-gateway, we're writing our own custom MetadataFetcher (working with the bucket index instead of scanning the bucket). Would be nice to expose the same metrics in the custom implementation too, so I'm wondering what's the sentiment to export FetcherMetrics, so that it could be reused by Cortex too.

Verification

N/A

@pracucci pracucci mentioned this pull request Dec 21, 2020
Merged
3 tasks
GiedriusS
GiedriusS reviewed Dec 23, 2020
View reviewed changes
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not ideologically opposed to this even though it feels a bit weird exposing the struct that holds the metrics. Sharing the metrics here for me means that we would be able to seamlessly switch between the metadata fetchers from the PoV of the user if needed in the future which is a nice thing. However, I'm curious to know the opinion of other people.

@pracucci
Copy link
Contributor Author

I'm not ideologically opposed to this even though it feels a bit weird exposing the struct that holds the metrics. Sharing the metrics here for me means that we would be able to seamlessly switch between the metadata fetchers from the PoV of the user if needed in the future which is a nice thing. However, I'm curious to know the opinion of other people.

Feel weird to me too. I'm wondering what could be a better approach. Currently I've copy-pasted the code into Cortex, but it's very error prone (eg. new label values added in the future), due to the usage of TxGaugeVec which requires all possible label values in input (these metrics are individually passed to MetadataFilter so on the Cortex side we have no easy way to know which are the possible values, unless we duplicate all filters code too).

I'm very open to better ideas.

@stale
Copy link

stale bot commented Feb 21, 2021

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Feb 21, 2021
bwplotka
bwplotka approved these changes Feb 25, 2021
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any problem with this, let's do it, reuse more!

@stale stale bot removed the stale label Feb 25, 2021
bwplotka
bwplotka reviewed Feb 25, 2021
View reviewed changes
pkg/block/fetcher.go
// FetcherMetrics holds metrics tracked by the metadata fetcher. This struct and its fields are exported
// to allow depending projects (eg. Cortex) to implement their own custom metadata fetcher while tracking
// compatible metrics.
type FetcherMetrics struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with this approach? 🤔

Potentially: Fetcher hasn't been designed to be replaced with a different implementation.

I think this the evolution of package that needs more implementations than one.

@pracucci
Export metadata fetcher metrics
f7e77d7 
Signed-off-by: Marco Pracucci <[email protected]>
@pracucci pracucci merged commit 0d86697 into thanos-io:master Feb 26, 2021
This was referenced Feb 26, 2021
Merged
Merged
andrejbranch pushed a commit to andrejbranch/thanos that referenced this pull request Mar 11, 2021
@pracucci
Export metadata fetcher metrics (thanos-io#3660)
d861898 
Signed-off-by: Marco Pracucci <[email protected]>
openshift-merge-robot pushed a commit to stolostron/thanos that referenced this pull request Apr 12, 2021
@songleo @squat
@pracucci @Namanl2001 @BradErz @GiedriusS @MalloZup @bwplotka @yeya24 @zecke @Biswajitghosh98 @mattjw79 @Hmerac @lilic @krya-kryak @SuperQ @metalmatze @soniasingla @kakkoyun @Abhishek357 @Sudhar287 @Max-Chandler @m-lawre @replay @hitanshu-mehta @s-urbaniak @simonpasquier @StephaneBrillant @saswatamcode @onprem
Upgrade thanos to release-0.19 (#8)
8460e86 
* CHANGELOG.md: add v0.16.0 link (thanos-io#3697)

This small commit fixes the changelog entry for release v0.16.0 so that
the section title includes a link to the actual release.

Signed-off-by: Lucas Servén Marín <[email protected]>

* Delete deletion-mark.json at last when deleting a block (thanos-io#3661)

* Delete deletion-mark.json at last when deleting a block

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed linter

Signed-off-by: Marco Pracucci <[email protected]>

* fixed broken links (thanos-io#3645)

Signed-off-by: Namanl2001 <[email protected]>

* chore(tutorials): fix some broken links in 2-lts (thanos-io#3702)

Signed-off-by: Bradley <[email protected]>

* Fix race condition in BinaryReader.LookupSymbol() (thanos-io#3705)

* Fix race condition in BinaryReader.LookupSymbol()

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed BinaryReader receiver and added CHANGELOG entry

Signed-off-by: Marco Pracucci <[email protected]>

* ui: make old bucket viewer UI work with vanilla blocks (thanos-io#3700)

* ui: make old UI work with vanilla blocks

Make the old bucket viewer UI work with vanilla Prometheus blocks by
checking whether the `Thanos` part exists before formatting the HTML.

Signed-off-by: Giedrius Statkevičius <[email protected]>

* CHANGELOG: update

Signed-off-by: Giedrius Statkevičius <[email protected]>

* ui/bucket: fix according to @squat's suggestions

Signed-off-by: Giedrius Statkevičius <[email protected]>

* ui: update bindata after latest changes

Signed-off-by: Giedrius Statkevičius <[email protected]>

* Query-frontend CMD: use detailed text description

Signed-off-by: dmaiocchi <[email protected]>
Co-authored-by: Bartlomiej Plotka <[email protected]>

* Upgrade Cortex and remove SwiftConfig (thanos-io#3708)

Signed-off-by: Marco Pracucci <[email protected]>

* chore(deps): upgrading hugo (v0.80.0) fixes thanos-io#3653 (thanos-io#3714)

Signed-off-by: Bradley <[email protected]>

* store: Make more S3 http.Transport settings configurable. (thanos-io#3657)

The http.Transport is not auto-tuning and one size does not seem to fit
all cases. In order to respond to a query a store gateway might need to
fetch a large (thousands) of postings, series and chunks.

While the number of idle connections has been increased recently it can
still be too low or expose bursty (opening, closing) behavior. Allow to
tune most of the http.Transport parameters. I considered embedding the
full Transport but there is already a Transport member.

Signed-off-by: Holger Hans Peter Freyther <[email protected]>

* added definiton of chunk and referenced it in the docs (thanos-io#3629)

* Update design.md

Update store.md

Update design.md

Update storage.md

Update troubleshooting.md

Signed-off-by: Biswajit Ghosh <[email protected]>

* Update design.md

Update store.md

Update design.md

Update troubleshooting.md

Update storage.md

Signed-off-by: Biswajit Ghosh <[email protected]>

* Upgraded to newest bingo. (thanos-io#3718)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Update help messages with a consistent use of capitals and periods. (thanos-io#3727)

* Update help messages with a consistent use of capitals and periods.

Signed-off-by: Matt Whitney <[email protected]>

* Update documentation to reflect message changes.

Signed-off-by: Matt Whitney <[email protected]>

* Run `make docs` to correct the documentation rather than manual edits.

Signed-off-by: Matt Whitney <[email protected]>

* docs: update examples/dashboards/dashboards.md (thanos-io#3733)

Signed-off-by: Mert Acikportali <[email protected]>

* pkg/errutil: correct the multierror file suffix (thanos-io#3736)

This commit changes the file suffix from .go.go to simply .go.

Signed-off-by: Lucas Servén Marín <[email protected]>

* Fixed website and added new step for release process (tmp). (thanos-io#3738)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* api: Added Proposal for Discovery Endpoint idea (InfoAPI) (thanos-io#3703)

* docs/proposals/210701_endpoint_discovery.md: Add initial proposal

Signed-off-by: Lili Cosic <[email protected]>

* Update docs/proposals/210701_endpoint_discovery.md

Co-authored-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Bartlomiej Plotka <[email protected]>

* Introduce "block-meta-fetch-concurrency" flag for compact and store (thanos-io#3752)

* Introduce "block-meta-fetch-concurrency" flag for compact and store

Furthermore consolidate the fetcher concurrency by moving the
fetcherConcurrency const to the fetcher code of the block package.

Signed-off-by: Johannes Frey <[email protected]>

* Document flags

Signed-off-by: Johannes Frey <[email protected]>

* Add `--query-range.request-downsampled` flag to Query Frontend (thanos-io#2641) (thanos-io#3723)

* Add `--query-range.request-downsampled` flag to Query Frontend (thanos-io#2641)

Signed-off-by: Vladimir Kononov <[email protected]>

* Apply suggestions from code review

Co-authored-by: Bartlomiej Plotka <[email protected]>
Signed-off-by: Vladimir Kononov <[email protected]>

Co-authored-by: Bartlomiej Plotka <[email protected]>

* Pinned to newer busybox on quay. (thanos-io#3762)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Fix panic on concurrent index-header lazy reader usage and unload (thanos-io#3760)

* Fix panic on concurrent index-header lazy reader usage and unload

Signed-off-by: Marco Pracucci <[email protected]>

* Addressed review comments

Signed-off-by: Marco Pracucci <[email protected]>

* Addressed review comments

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed Planner.Plan() description (thanos-io#3767)

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed panic on concurrent index-header lazy load / unload (thanos-io#3759)

Signed-off-by: Marco Pracucci <[email protected]>

* Pad compaction planner size check (thanos-io#3773)

Pad the index size check in the compaction planner by 15% to avoid
situations where the sum of index bytes bloats to be larger than the
sum of the original index sizes.

* thanos-io#3724
* thanos-io#3750

Signed-off-by: Ben Kochie <[email protected]>

* pkg/server/http: Allow passing http.ServeMux with a server option (thanos-io#3769)

Signed-off-by: Matthias Loibl <[email protected]>

* Update Katacoda Official Link (thanos-io#3768)

Signed-off-by: soniasingla <[email protected]>

* Nomitate kakkoyun for the v0.20.0 release (thanos-io#3778)

Signed-off-by: Kemal Akkoyun <[email protected]>

* Fix typo in query component documentation for store api link (thanos-io#3675)

* Fix wrong store api link

Signed-off-by: Junyoung, Sung <[email protected]>

* Fix store api link to absolute

Signed-off-by: Junyoung, Sung <[email protected]>

* e2e: Resolved Flaky tests (thanos-io#3777)

* added retry function

Signed-off-by: Abhishek357 <[email protected]>

* fixed CI test

Signed-off-by: Abhishek357 <[email protected]>

* restructured code

Signed-off-by: Abhishek357 <[email protected]>

* Add an exempt label to ignore issue that has a PR

Signed-off-by: Kemal Akkoyun <[email protected]>

* in memory cache for caching bucket (thanos-io#3579)

* in memeory cache for caching bucket

Signed-off-by: Sudharshann D <[email protected]>

* addressing comments

Signed-off-by: Sudharshann D <[email protected]>

* Introduce "allow overlapping blocks" flag for Thanos receiver (thanos-io#3792)

* add tsdb.allow-overlapping-blocks flag to receiver

Signed-off-by: Max Chandler <[email protected]>

* make format

Signed-off-by: Max Chandler <[email protected]>

* cleanup whitespace

Signed-off-by: Max Chandler <[email protected]>

* cleanup whitespace

Signed-off-by: Max Chandler <[email protected]>

* make docs

Signed-off-by: Max Chandler <[email protected]>

* fix changelog conflict

Signed-off-by: Max Chandler <[email protected]>

Co-authored-by: Matt Lawrence <[email protected]>

* Reduce memory allocations in bucketBlock.readChunkRange() (thanos-io#3796)

Signed-off-by: Marco Pracucci <[email protected]>

* update prometheus version

Signed-off-by: Mauro Stettler <[email protected]>

* update cortex

Signed-off-by: Mauro Stettler <[email protected]>

* add new property to LabelValuesRequest

Signed-off-by: Mauro Stettler <[email protected]>

* set label matchers in LabelValueRequest

Signed-off-by: Mauro Stettler <[email protected]>

* make docs

Signed-off-by: Mauro Stettler <[email protected]>

* Allow to customise S3 SSE on a per-request basis (thanos-io#3783)

* Allow to customise S3 SSE on a per-request basis

Signed-off-by: Marco Pracucci <[email protected]>

* Addressed review comments

Signed-off-by: Marco Pracucci <[email protected]>

* Refactoring: allow to pass BytesPool to store.NewBucketStore() (thanos-io#3801)

Signed-off-by: Marco Pracucci <[email protected]>

* Allow to customise the partitioner used by the BucketStore (thanos-io#3802)

Signed-off-by: Marco Pracucci <[email protected]>

* Fix button display when there is no panels (thanos-io#3694)

Signed-off-by: Namanl2001 <[email protected]>

* fix tests

Signed-off-by: Mauro Stettler <[email protected]>

* Allow downstream projects to customise the Partitioner (thanos-io#3808)

Signed-off-by: Marco Pracucci <[email protected]>

* Makefile: Fix to command to find React source files (thanos-io#3805)

Signed-off-by: Hitanshu Mehta <[email protected]>

* Merge release 0.18 (thanos-io#3809)

* CHANGELOG.md: release v0.18.0

Signed-off-by: Lucas Servén Marín <[email protected]>

* VERSION,tutorials: bump versions

Signed-off-by: Lucas Servén Marín <[email protected]>

* Revert "VERSION,tutorials: bump versions"

This reverts commit 5a27c70.
The "thanos:" prefix for the container images was accidentally removed.

Signed-off-by: Lucas Servén Marín <[email protected]>

* VERSION,tutorials: bump version for all images

Signed-off-by: Lucas Servén Marín <[email protected]>

* CHANGELOG: bump release date

Signed-off-by: Lucas Servén Marín <[email protected]>

* pkg/rules/proxy: fix hotlooping when receiving client errors

Currently, if we receive an error from the underlying client stream,
we continue with trying to receive additional data.
This causes a hotloop as we will receive the same error again.
This fixes it by returning in the error case and adds a unit test for the proxy logic.

Fixes thanos-io#3717

Signed-off-by: Sergiusz Urbaniak <[email protected]>

* CHANGELOG.md: fix changelog to incorporate new fix (thanos-io#3737)

This commit fixes the order of the changelog to properly reflect a
recent change.

Signed-off-by: Lucas Servén Marín <[email protected]>

* Fixed website and added new step for release process (tmp). (thanos-io#3738)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* CHANGELOG.md: update v0.18.0 release date

Signed-off-by: Lucas Servén Marín <[email protected]>

Co-authored-by: Sergiusz Urbaniak <[email protected]>
Co-authored-by: Bartlomiej Plotka <[email protected]>

* reloader: try to fix test flakiness (thanos-io#3798)

This is an attempt of fixing the `TestReloader_DirectoriesApply` test
flakiness. I call this an attempt because I cannot reproduce it locally.
However, I have noticed that during runs where this fails the logs look
like this:

```
--- FAIL: TestReloader_DirectoriesApply (3.04s)
    reloader_test.go:256: Performing step number 0
    reloader_test.go:256: Performing step number 1
    reloader_test.go:256: Performing step number 2
    reloader_test.go:256: Performing step number 3
    reloader_test.go:256: Performing step number 4
    reloader_test.go:256: Performing step number 6
    reloader_test.go:343: reloader_test.go:343:

        	exp: 5

        	got: 6
```

It immediately jumps to another value. This gave me a hint and I think
that this is happening because potentially `i` can be written to/read
from by multiple goroutines. On very resource constrained systems like
the CircleCI runners, it could just happen that the `if` doesn't do what
it is supposed to.

Try to fix this problem by protecting the whole HTTP handler with a
mutex. Since this is only a test and not a performance critical path, I
think this is a reasonable change to do.

Add extra check for reload failures.

Signed-off-by: Giedrius Statkevičius <[email protected]>

* addressing PR comments

Signed-off-by: Mauro Stettler <[email protected]>

* Block Viewer: Move overlapping blocks to separate rows (thanos-io#3729)

* Block Viewer: overlapping blocks issue resolved

Signed-off-by: Namanl2001 <[email protected]>

* small changes and make assets

Signed-off-by: Namanl2001 <[email protected]>

* Add flag to set default step

Signed-off-by: Hitanshu Mehta <[email protected]>

* Add support to use default step in classic ui

Signed-off-by: Hitanshu Mehta <[email protected]>

* Add support to use default step in new ui

Signed-off-by: Hitanshu Mehta <[email protected]>

* Lint fixes

Signed-off-by: Hitanshu Mehta <[email protected]>

* change ParseStep function to be consistent with ui

Signed-off-by: Hitanshu Mehta <[email protected]>

* Improve description of query.default-step flag

Signed-off-by: Hitanshu Mehta <[email protected]>

* Minor fixes

Signed-off-by: Hitanshu Mehta <[email protected]>

* Use default value when  flag is undefined

Signed-off-by: Hitanshu Mehta <[email protected]>

* minor fixes

Signed-off-by: Hitanshu Mehta <[email protected]>

* Minor fixes

Signed-off-by: Hitanshu Mehta <[email protected]>

* Update changelog

Signed-off-by: Hitanshu Mehta <[email protected]>

* Fix typo

Signed-off-by: Hitanshu Mehta <[email protected]>

* Receive: Improve handling of empty time series from clients (thanos-io#3815)

* exit early if a request has no timeseries data

Signed-off-by: Matt Lawrence <[email protected]>
Co-authored-by: Max Chandler <[email protected]>

* update changelog

Signed-off-by: Matt Lawrence <[email protected]>

* update changelog

Signed-off-by: Matt Lawrence <[email protected]>

* Move constant to right side of comparison

Co-authored-by: Lucas Servén Marín <[email protected]>
Signed-off-by: Matt Lawrence <[email protected]>

Co-authored-by: Max Chandler <[email protected]>
Co-authored-by: Lucas Servén Marín <[email protected]>

* Reduced allocated memory by chunks reader in the store gateway at query time (thanos-io#3814)

* Reduced allocated memory by chunks reader in the store gateway at query time

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed linter issues

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed linter (hopefully)

Signed-off-by: Marco Pracucci <[email protected]>

* Renamed function

Signed-off-by: Marco Pracucci <[email protected]>

* Updated comment

Signed-off-by: Marco Pracucci <[email protected]>

* Updated code comment

Signed-off-by: Marco Pracucci <[email protected]>

* promclient: fix error's message (thanos-io#3824)

Use the provided `method` in the error messages. I got scared reading
the logs that the requests are still being sent using GET instead of
POST which I had specified.

Signed-off-by: Giedrius Statkevičius <[email protected]>

* pkg/ui/react-app: update dependencies (thanos-io#3818)

Fix lodash security issue.

Signed-off-by: Simon Pasquier <[email protected]>

* Add objstore.List() recursive support (thanos-io#3823)

* Add objstore.List() recursive support

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed CachingBucket

Signed-off-by: Marco Pracucci <[email protected]>

* Fix Cortex compilation issue

Signed-off-by: Marco Pracucci <[email protected]>

* Tidy go.mod/sum

Signed-off-by: Marco Pracucci <[email protected]>

* Fix linter

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed TestMetricBucket_Close

Signed-off-by: Marco Pracucci <[email protected]>

* Upgraded Cortex

Signed-off-by: Marco Pracucci <[email protected]>

* Inlined struct initialisation

Signed-off-by: Marco Pracucci <[email protected]>

* Upgrade Cortex again

Signed-off-by: Marco Pracucci <[email protected]>

* Upgrade Cortex (thanos-io#3828)

Signed-off-by: Marco Pracucci <[email protected]>

* Truncated S3 "get object" response is reported as error (thanos-io#3795)

* Add test to ensure S3 truncated response is an error

Signed-off-by: Marco Pracucci <[email protected]>

* Added missing copyright

Signed-off-by: Marco Pracucci <[email protected]>

* Upgraded Minio

Signed-off-by: Marco Pracucci <[email protected]>

* Upgraded Minio again

Signed-off-by: Marco Pracucci <[email protected]>

* Implement federated metric metadata API (thanos-io#3686)

* support federated metadata API

Signed-off-by: Ben Ye <[email protected]>

* update comments

Signed-off-by: yeya24 <[email protected]>

* use parseInt

Signed-off-by: yeya24 <[email protected]>

* address Prem's comments

Signed-off-by: yeya24 <[email protected]>

* update proto comment

Signed-off-by: yeya24 <[email protected]>

* add changelog

Signed-off-by: yeya24 <[email protected]>

* Fixed TestBucketStore_ManyParts_e2e (thanos-io#3841)

https://app.circleci.com/pipelines/github/thanos-io/thanos/5120/workflows/efd2a21d-13b7-4035-99e3-cb1af8023694/jobs/13809

This fail is only visible for anyone from Thanos Team proposing PR, due
to extra tests against bucket providers.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Reinforcing errutil and fixed ugly bug which caused empty []error to te treated as error, (thanos-io#3836)

Previous multi-error implementation could cause very ugly bug of returnig empty multi-error
that should be treated as success not error by API, but if .Err() is not invoked it will be
used as non nil error.

Once we merge this, we can do cleaner solution that slighly change nesting behaviour: thanos-io#3833

There were 9 places where we had this bug in handler due to MultiError lib allowing to do so.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Export metadata fetcher metrics (thanos-io#3660)

Signed-off-by: Marco Pracucci <[email protected]>

* store: Cleaned up API for test/benchmark purposes. (thanos-io#3650)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Allow Cortex to fully reuse FetcherMetrics (thanos-io#3842)

Signed-off-by: Marco Pracucci <[email protected]>

* downsample: ensure consistent order (thanos-io#3843)

Ensure that we have a consistent order of blocks that we are going to
downsample. `range` over maps doesn't enforce any particular order on
purpose.

This is needed for https://github.com/thanos-io/thanos/pull/3031/files.
ATM in that PR before downsampling we delete all directories which do
not match blocks ULIDs in the remote object storage. Ideally, we should
only keep around the files of a block which we are about to downsample.

It is impossible to do that properly ATM if during another iteration
we'd start from a different block. Thus, let's have a consistent order.

Signed-off-by: Giedrius Statkevičius <[email protected]>

* [Fix] Replace unsupported %v macro in logs (thanos-io#3847)

Signed-off-by: Stéphane Brillant <[email protected]>

* Update ui npm dependencies to latest versions (thanos-io#3813)

* Update ui npm deps to latest

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix linting errors

Signed-off-by: Saswata Mukherjee <[email protected]>

* Change camelcase lint rule

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix CI configs and scripts to work with main branch (thanos-io#3848)

Signed-off-by: Prem Saraswat <[email protected]>

* block: precalculate hashes if enabled and use them during compaction (downloading) (thanos-io#3031)

* block: precalculate hashes if enabled and use them during compaction

Added the possibility to ignore certain directories in
objstore.{Download,DownloadDir}. Do not download files which have the
same hash as in remote object storage. Wire up `--hash-func` so that
writers could specify what hash function to use when uploading. There is
no performance impact if no hash function has been explicitly specified.
Clean up the removal of files logic in Thanos Compact to ensure we do
not remove something that exists on disk already.

Tested manually + new tests cover all of this more or less.

Signed-off-by: Giedrius Statkevičius <[email protected]>

* block: expose GatherFileStats and use it

Signed-off-by: Giedrius Statkevičius <[email protected]>

* Revert "block: expose GatherFileStats and use it"

This reverts commit 259c70b.

Signed-off-by: Giedrius Statkevičius <[email protected]>

* block: do not calc hash for dirs, add locks

Signed-off-by: Giedrius Statkevičius <[email protected]>

* docs/tools: update

Signed-off-by: Giedrius Statkevičius <[email protected]>

* shipper: pass s.hashFunc

Signed-off-by: Giedrius Statkevičius <[email protected]>

* Fix according to Bartek's comments

Signed-off-by: Giedrius Statkevičius <[email protected]>

* compact: clean up comment

Signed-off-by: Giedrius Statkevičius <[email protected]>

* block: close with log on error

Signed-off-by: Giedrius Statkevičius <[email protected]>

* *: remove unused FNs

Signed-off-by: Giedrius Statkevičius <[email protected]>

* compact: add e2e test for new hash functionality

Signed-off-by: Giedrius Statkevičius <[email protected]>

* Fix according to Bartek's comments

Signed-off-by: Giedrius Statkevičius <[email protected]>

* mixin: Upgrade jsonnet tooling (thanos-io#3855)

* Upgrade jsonnet tooling

Signed-off-by: Kemal Akkoyun <[email protected]>

* Fix bingo version issue

Signed-off-by: Kemal Akkoyun <[email protected]>

* Fix unintended removal issue

Signed-off-by: Kemal Akkoyun <[email protected]>

* Update reference of master to main in docs (thanos-io#3849)

* Update reference of master to main in docs

Signed-off-by: Prem Saraswat <[email protected]>

* Update image tag in tutorials/kubernetes-helm

Signed-off-by: Prem Saraswat <[email protected]>

* Cut v0.19.0-rc.0 (thanos-io#3860)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Fixed missed changelog comments. (thanos-io#3865)

Missed from: thanos-io#3860

Signed-off-by: Bartlomiej Plotka <[email protected]>

* rename Metadata API to MetricMetadata API (thanos-io#3877)

Signed-off-by: yeya24 <[email protected]>

* Fix parseStep bug introduced in PR thanos-io#3740 (thanos-io#3887) (thanos-io#3889)

* Fix bug introduced in PR thanos-io#3740

Signed-off-by: Hitanshu Mehta <[email protected]>

* Minor change in test

Signed-off-by: Hitanshu Mehta <[email protected]>

Co-authored-by: Hitanshu Mehta <[email protected]>

* Tools: rewrite delete can delete series only if it matches all matchers (thanos-io#3886) (thanos-io#3890)

* rewrite delete should delete a series only if it matches all matchers in a deletion request

Signed-off-by: yeya24 <[email protected]>

* add test case

Signed-off-by: yeya24 <[email protected]>

Co-authored-by: Ben Ye <[email protected]>

* cmd/thanos/receive.go: Receive client infers TLS (thanos-io#3899)

Currently, the thanos Receiver infers whether it should use TLS for
the gRPC clients that forwards time series to other receivers based on
whether the remote-write HTTP server uses TLS. This is not correct, as
the HTTP server may use TLS without the gRPC server using TLS. This
commit fixes the inference.

Longer-term, this will be taken care of by the receive/router split.

Signed-off-by: Lucas Servén Marín <[email protected]>

* Cut v0.19.0-rc.1 (thanos-io#3900)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* tools: Fix partial and empty matchers in rewrite (thanos-io#3891)

* Fix partial and empty matchers match

Signed-off-by: yeya24 <[email protected]>

* add testcase for non-equal matchers

Signed-off-by: yeya24 <[email protected]>

* v0.19.0 patch: Added receive benchmark; Fixed Receiver excessive mem usage introduced in 0.17 (thanos-io#3943)

* Added receive benchmark, baseline.

```
goos: linux
goarch: amd64
pkg: github.com/thanos-io/thanos/pkg/receive
BenchmarkHandlerReceiveHTTP
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them.
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them./OK
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them./OK-12      	   22260	   1550152 ns/op	 1380340 B/op	    6093 allocs/op
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them./conflict_errors
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them./conflict_errors-12         	    6619	   6430408 ns/op	 4522487 B/op	   26118 allocs/op
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them.
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them./OK
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them./OK-12                     	    2695	  17208794 ns/op	15072963 B/op	   60441 allocs/op
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them./conflict_errors
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them./conflict_errors-12        	     474	  72533286 ns/op	46396932 B/op	  260141 allocs/op
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12                	     270	 137050518 ns/op	226595379 B/op	     132 allocs/op
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12   	      21	1616025443 ns/op	698724321 B/op	     408 allocs/op
PASS

Process finished with exit code 0
```


Signed-off-by: Bartlomiej Plotka <[email protected]>

* Copy labels.

```
GOROOT=/home/bwplotka/.gvm/gos/go1.15 #gosetup
GOPATH=/home/bwplotka/Repos/thanosgopath #gosetup
/home/bwplotka/.gvm/gos/go1.15/bin/go test -c -o /tmp/___BenchmarkHandlerReceiveHTTP_in_github_com_thanos_io_thanos_pkg_receive github.com/thanos-io/thanos/pkg/receive #gosetup
/tmp/___BenchmarkHandlerReceiveHTTP_in_github_com_thanos_io_thanos_pkg_receive -test.v -test.bench ^\QBenchmarkHandlerReceiveHTTP\E$ -test.run ^$ -test.benchmem -test.benchtime=30s
goos: linux
goarch: amd64
pkg: github.com/thanos-io/thanos/pkg/receive
BenchmarkHandlerReceiveHTTP
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12      	   25887	   1537262 ns/op	 1380023 B/op	    6092 allocs/op
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12         	    4237	   7547968 ns/op	 4522583 B/op	   26118 allocs/op
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12                     	    2205	  16513380 ns/op	15071092 B/op	   60420 allocs/op
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors
BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12        	     525	  67278233 ns/op	46396645 B/op	  260141 allocs/op
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12               	     285	 148049189 ns/op	226596168 B/op	     132 allocs/op
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors
BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12  	      20	1731361499 ns/op	698722550 B/op	     401 allocs/op
PASS

Process finished with exit code 0

```

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Addded bench.,

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Fix.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Improved API.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Changelog.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Addressed Lucas comments.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* compact: clean up directories thoroughly (thanos-io#3869)

* compact: clean up directories properly

I couldn't stop thinking about this code for some reason and I have
figured that I had missed one case in thanos-io#3031. We need to also clean up
the directories in compaction groups. A compaction could fail leaving
some new directory with a random ULID on the disk. Before attempting to
do another compaction loop, we need to remove it as well because the
compaction process always produces a new, unique directory.

Signed-off-by: Giedrius Statkevičius <[email protected]>

* Remove all non expected dirs.

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Addressed comment.

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Bartlomiej Plotka <[email protected]>

* Added benchmark, Moved minio-deps to fork without race fix we don't need. (thanos-io#3968)

Fixes: thanos-io#3917

Long term fix: thanos-io#3967

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Cut v0.19.0-rc.2 (thanos-io#3969)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* pkg/rules: fix deduplication of equal alerts with different labels (thanos-io#3960) (thanos-io#3999)

Currently, if an alerting rule having the same name with different
severity labels is being returned from different replicas then they
are being treated as separate alerts.

Given the following alerts a1,a2 with severities s1,s2 returned from
replicas r1,2:

a1[s1,r1]
a1[s2,r1]
a1[s1,r2]
a1[s2,r2]

Then, currently, the algorithm deduplicates to:

a1[s1]
a1[s2]
a1[s1]
a1[s2]

Instead of the intendet result:

a1[s1]
a1[s2]

This fixes it by removing replica labels before sorting labels for
deduplication.

Signed-off-by: Sergiusz Urbaniak <[email protected]>
# Conflicts:
#	CHANGELOG.md

Co-authored-by: Sergiusz Urbaniak <[email protected]>

* Cut v0.19.0 (thanos-io#3998)

Signed-off-by: Bartlomiej Plotka <[email protected]>

* Fixed VERSION file.

Signed-off-by: Bartlomiej Plotka <[email protected]>

Co-authored-by: Lucas Servén Marín <[email protected]>
Co-authored-by: Marco Pracucci <[email protected]>
Co-authored-by: Naman Lakhwani <[email protected]>
Co-authored-by: Bradley <[email protected]>
Co-authored-by: Giedrius Statkevičius <[email protected]>
Co-authored-by: dmaiocchi <[email protected]>
Co-authored-by: Bartlomiej Plotka <[email protected]>
Co-authored-by: Ben Ye <[email protected]>
Co-authored-by: Holger Freyther <[email protected]>
Co-authored-by: Biswajit Ghosh <[email protected]>
Co-authored-by: Matt W <[email protected]>
Co-authored-by: Mert Açıkportalı <[email protected]>
Co-authored-by: Lili Cosic <[email protected]>
Co-authored-by: Johannes Frey <[email protected]>
Co-authored-by: Vladimir Kononov <[email protected]>
Co-authored-by: Ben Kochie <[email protected]>
Co-authored-by: Matthias Loibl <[email protected]>
Co-authored-by: Sonia Singla <[email protected]>
Co-authored-by: Kemal Akkoyun <[email protected]>
Co-authored-by: Junyoung, Sung <[email protected]>
Co-authored-by: Abhishek Singh Chauhan <[email protected]>
Co-authored-by: Sudhar287 <[email protected]>
Co-authored-by: Max Chandler <[email protected]>
Co-authored-by: Matt Lawrence <[email protected]>
Co-authored-by: Mauro Stettler <[email protected]>
Co-authored-by: Hitanshu Mehta <[email protected]>
Co-authored-by: Sergiusz Urbaniak <[email protected]>
Co-authored-by: Hitanshu Mehta <[email protected]>
Co-authored-by: Matt Lawrence <[email protected]>
Co-authored-by: Max Chandler <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Stephane Brillant <[email protected]>
Co-authored-by: Saswata Mukherjee <[email protected]>
Co-authored-by: Prem Saraswat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwplotka bwplotka bwplotka approved these changes

@GiedriusS GiedriusS GiedriusS left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pracucci @GiedriusS @bwplotka