Upgrade Thanos and Prometheus #2604

codesome · 2020-05-19T08:39:55Z

Signed-off-by: Ganesh Vernekar [email protected]

What this PR does:

Upgrade Thanos to the commit 806479182a6b which also upgrades Prometheus to cd73b3d33e064bbd846fc7a26dc8c313d46af382 which is between 2.17.x and 2.18.x.

Note: go.etcd.io/etcd was upgraded to @master without which it causes issues with grpc upgrade.

Which issue(s) this PR fixes:
Fixes #

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

codesome · 2020-05-20T07:40:11Z

Tests are passing now and it is ready for review.

One of the changes to note is that the flag promql.lookback-delta was being initialized in both querier and ruler to a global LookbackDelta in promql package. Now that variable is a part of promql Engine options and not longer a global. Since Ruler takes the engine created by querier in initRuler, LookbackDelta is a part of querier config now with the same flag, and Ruler takes the engine created by Querier as before.

codesome · 2020-05-20T07:41:22Z

Does this require a changelog entry? There are some enhancements that come Prometheus upgrade, but no specific cortex changes.

codesome · 2020-05-20T07:51:03Z

Added changelog entry for the upgrade and also an addition of config file option as described above.

Signed-off-by: Ganesh Vernekar <[email protected]>

pracucci

Good job Ganesh! I think this PR is in a pretty good state, but I've left few comments that I would like you to address/discuss before moving forward.

I've also checked the updated dependencies, and these LGTM:

github.com/aws/aws-sdk-go from v1.27.0 to v1.29.18: checked the CHANGELOG, nothing affecting us
github.com/hashicorp/consul/api from v1.3.0 to v1.4.0: checked the CHANGELOG, nothing affecting us
github.com/prometheus/client_golang from v1.5.0 to v1.5.1: fixed "superfluous WriteHeader call" log
go.etcd.io/bbolt from v1.3.3 to v1.3.4: checked the releases page, minor fixes not affecting us
go.uber.org/atomic from v1.5.1 to v1.6.0: just dropped a dependency
google.golang.org/grpc from v1.26.0 to v1.27.1: checked the releases page, no change that should directly affect us
Thanos: minor changes, not relevant to us

I'm a bit more concerned about the etcd upgrade. Could you check if there's anything we should be aware of, please?

pkg/ingester/ingester_v2.go

pkg/querier/block.go

pkg/querier/blocks_store_balanced_set.go

pkg/querier/blocks_store_queryable.go

pkg/querier/chunk_store_queryable.go

pkg/querier/querier.go

pkg/ruler/compat.go

pkg/storegateway/bucket_stores_test.go

pkg/api/api.go

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome · 2020-05-22T07:36:25Z

etcd is now upgraded to v3.4.9 (it was at v3.4.3 before). go.mod says v0.5.0-alpha.5.0.20200520232829-54ba9589114f, but 54ba9589114f is the last commit in release-3.4 branch (at the time of this upgrade) which is at v3.4.9.

pstibrany

Changes in Cortex and vendored Thanos look good. I haven't checked all updated dependencies though.

pkg/ingester/ingester_v2.go

pkg/querier/querier.go

pkg/storegateway/gateway_test.go

Signed-off-by: Ganesh Vernekar <[email protected]>

pracucci

All good, but I've a doubt the Select(false in the ingester v2.

pkg/querier/blocks_store_balanced_set.go

pracucci · 2020-05-22T09:07:17Z

pkg/ingester/ingester_v2.go

 	defer q.Close()

-	ss, err := q.Select(matchers...)
+	ss, _, err := q.Select(false, nil, matchers...)


@pstibrany doesn't the querier expects sorted series?

This is correct. Querier will need to merge series from different ingesters anyway (based on fingerprint), and will then sort them.

If it was coming sorted from ingester, querier would have easier time merging, but on the other hand, it would use more CPU time in ingesters as well. We want to disturb ingesters as little as possible on read path.

Signed-off-by: Ganesh Vernekar <[email protected]>

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

pracucci

LGTM! Thanks for the hard work, amazing job! 🎉

… by a buffer incorrectly reused while iterating samples. Signed-off-by: Marco Pracucci <[email protected]>

* Fixed incorrect query results introduced in #2604 caused by a buffer incorrectly reused while iterating samples. Signed-off-by: Marco Pracucci <[email protected]> * Updated prometheus Signed-off-by: Marco Pracucci <[email protected]> * Update pkg/querier/blocks_store_queryable_test.go Signed-off-by: Marco Pracucci <[email protected]> Co-authored-by: Peter Štibraný <[email protected]> * Update pkg/querier/blocks_store_queryable_test.go Signed-off-by: Marco Pracucci <[email protected]> Co-authored-by: Peter Štibraný <[email protected]> * Fixed linter Signed-off-by: Marco Pracucci <[email protected]> * Updated CHANGELOG entry Signed-off-by: Marco Pracucci <[email protected]> Co-authored-by: Peter Štibraný <[email protected]>

pull-request-size bot added the size/XXL label May 19, 2020

codesome force-pushed the thanos-master branch 2 times, most recently from 3f35bc3 to 955cbbe Compare May 19, 2020 14:22

codesome marked this pull request as ready for review May 19, 2020 15:57

codesome force-pushed the thanos-master branch 3 times, most recently from d9460af to e5c6986 Compare May 20, 2020 07:03

codesome force-pushed the thanos-master branch from e5c6986 to 4e06cd7 Compare May 20, 2020 07:50

codesome force-pushed the thanos-master branch from 4e06cd7 to 9cc5f91 Compare May 20, 2020 08:12

Upgrade Thanos and Prometheus

e3eaf0d

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the thanos-master branch from 9cc5f91 to e3eaf0d Compare May 20, 2020 08:53

pracucci reviewed May 21, 2020

View reviewed changes

codesome force-pushed the thanos-master branch 2 times, most recently from aafb5b3 to fb84354 Compare May 21, 2020 14:21

Fix review comments

1998678

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the thanos-master branch from fb84354 to 1998678 Compare May 22, 2020 05:56

Ganesh Vernekar added 2 commits May 22, 2020 11:41

Merge remote-tracking branch 'upstream/master' into thanos-master

357c06b

Signed-off-by: Ganesh Vernekar <[email protected]>

Fix review comments

c85b5c9

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the thanos-master branch from 401e70f to c85b5c9 Compare May 22, 2020 07:06

pstibrany approved these changes May 22, 2020

View reviewed changes

Move etcd to v3.4.9 and address reviews

40a13d4

Signed-off-by: Ganesh Vernekar <[email protected]>

codesome force-pushed the thanos-master branch from 30f3cbc to 40a13d4 Compare May 22, 2020 08:48

Fix lint

87d52d0

Signed-off-by: Ganesh Vernekar <[email protected]>

pracucci reviewed May 22, 2020

View reviewed changes

Ganesh Vernekar and others added 2 commits May 22, 2020 14:56

Reviews

6e7118e

Signed-off-by: Ganesh Vernekar <[email protected]>

Update pkg/querier/blocks_store_balanced_set.go

a2623fc

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

pracucci approved these changes May 22, 2020

View reviewed changes

pracucci merged commit b40c117 into cortexproject:master May 22, 2020

pracucci mentioned this pull request Jun 8, 2020

Fixed incorrect query results introduced in #2604 #2697

Merged

3 tasks

pracucci added a commit to pracucci/cortex that referenced this pull request Jun 9, 2020

Fixed incorrect query results introduced in cortexproject#2604 caused…

dfe5eaa

… by a buffer incorrectly reused while iterating samples. Signed-off-by: Marco Pracucci <[email protected]>

Upgrade Thanos and Prometheus #2604

Upgrade Thanos and Prometheus #2604

Uh oh!

Conversation

codesome commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesome commented May 20, 2020

Uh oh!

codesome commented May 20, 2020

Uh oh!

codesome commented May 20, 2020

Uh oh!

pracucci left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codesome commented May 22, 2020

Uh oh!

pstibrany left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pracucci left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pracucci May 22, 2020

Choose a reason for hiding this comment

Uh oh!

pstibrany May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pstibrany May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pracucci left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codesome commented May 19, 2020 •

edited

Loading

pstibrany May 22, 2020 •

edited

Loading

pstibrany May 22, 2020 •

edited

Loading