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

*: update Prometheus / promql-engine dependency with fix + add test case for pushed down subquery #5602

Closed
wants to merge 4 commits into from
Closed

*: update Prometheus / promql-engine dependency with fix + add test case for pushed down subquery #5602

wants to merge 4 commits into from

Conversation

GiedriusS
Copy link
Member

Repro for #5600.

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

@GiedriusS GiedriusS mentioned this pull request Aug 17, 2022
Merged
GiedriusS
GiedriusS commented Aug 17, 2022
View reviewed changes
go.mod Outdated
@@ -246,6 +246,8 @@ replace (
// Required by Cortex https://github.com/cortexproject/cortex/pull/3051.
github.com/bradfitz/gomemcache => github.com/themihai/gomemcache v0.0.0-20180902122335-24332e2d58ab

github.com/prometheus/prometheus => github.com/vinted/prometheus v1.8.2-0.20220817132046-4af7e615cfb4
Copy link
Member Author

Choose a reason for hiding this comment

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

Temporary fork until prometheus/prometheus#11163 is figured out.

fpetkovski
fpetkovski previously approved these changes Aug 18, 2022
View reviewed changes
Copy link
Contributor

@fpetkovski fpetkovski left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm.

We can also wait a few days see if your upstream patch gets merged in Prometheus.

@GiedriusS
e2e: add tests for pushed down subqueries
84d22a1 
Repro for thanos-io#5600.

Signed-off-by: Giedrius Statkevičius <[email protected]>
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 4, 2022
@GiedriusS GiedriusS changed the title e2e: add tests for pushed down subqueries *: update Prometheus / promql-engine dependency with fix + add test case for pushed down subquery Oct 4, 2022
@GiedriusS
*: update dependency
b98f5ae 
Update prometheus dependency.

Signed-off-by: Giedrius Statkevičius <[email protected]>
@GiedriusS
test: fix test
39ea486 
Signed-off-by: Giedrius Statkevičius <[email protected]>
@fpetkovski
Copy link
Contributor

Hm, is the test failure relevant?

@GiedriusS
Copy link
Member Author

Hm, is the test failure relevant?

Yeah, something went wrong. Looking into it

@yeya24
Copy link
Contributor

yeya24 commented Oct 5, 2022

This contains the new OOO samples feature in TSDB?

@douglascamata
Copy link
Contributor

FYI Prometheus has tagged v0.29.0 just few mins ago (I was about to open a PR).

@GiedriusS
Copy link
Member Author

Feel free to update it in a separate PR as I'll be on vacation for a bit

@matej-g
Copy link
Collaborator

matej-g commented Oct 6, 2022

Looks like this is the culprit, because of change here https://github.com/jesusvazquez/prometheus/blob/c785aa0b69c2aab6966209f91d5294fb993e979e/tsdb/wal/wal.go#L282 coming from prometheus/prometheus#11075:

panic: duplicate metrics collector registration attempted

goroutine 4571 [running]:
github.com/prometheus/client_golang/prometheus.(*wrappingRegisterer).MustRegister(0xc0000acc60, {0xc000305ea0?, 0x7, 0x0?})
	/home/circleci/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/wrap.go:106 +0x151
github.com/prometheus/prometheus/tsdb/wal.newWALMetrics({0x145ef58, 0xc0000acc60})
	/home/circleci/go/pkg/mod/github.com/prometheus/[email protected]/tsdb/wal/wal.go:238 +0x64e
github.com/prometheus/prometheus/tsdb/wal.NewSize({0x1457080, 0xc000120500}, {0x145fb28?, 0xc002d22660}, {0xc000626800, 0x3b}, 0x8000000, 0x0)
	/home/circleci/go/pkg/mod/github.com/prometheus/[email protected]/tsdb/wal/wal.go:282 +0x2e5
github.com/prometheus/prometheus/tsdb.open({0xc000626680, 0x37}, {0x1457080, 0xc000120500}, {0x145fb28?, 0xc002d22660}, 0xc000700480, {0xc000048e60, 0xa, 0xa}, ...)
	/home/circleci/go/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:750 +0x94e
github.com/prometheus/prometheus/tsdb.Open({0xc000626680, 0x37}, {0x1457080, 0xc000120500}, {0x145fb28, 0xc002d22660}, 0x28?, 0x43d147?)
	/home/circleci/go/pkg/mod/github.com/prometheus/[email protected]/tsdb/db.go:613 +0x9b
github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).startTSDB(0xc00252c1e0, {0x1457080, 0xc000120500}, {0x113bf28, 0x3}, 0x0?)
	/home/circleci/project/pkg/receive/multitsdb.go:448 +0x42e
github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant.func1()
	/home/circleci/project/pkg/receive/multitsdb.go:510 +0x4d
created by github.com/thanos-io/thanos/pkg/receive.(*MultiTSDB).getOrLoadTenant
	/home/circleci/project/pkg/receive/multitsdb.go:509 +0x32b

@douglascamata douglascamata mentioned this pull request Oct 6, 2022
Merged
2 tasks
@GiedriusS GiedriusS closed this Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpetkovski fpetkovski fpetkovski left review comments

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@GiedriusS @fpetkovski @yeya24 @douglascamata @matej-g