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

*: bump prometheus and fix fallout #6772

Merged
merged 1 commit into from Oct 6, 2023
Merged

*: bump prometheus and fix fallout #6772

merged 1 commit into from Oct 6, 2023

Conversation

MichaHoffmann
Copy link
Contributor

@MichaHoffmann MichaHoffmann commented Oct 4, 2023 •
edited

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

Changes

  • bump prometheus to latest main
  • fix much fallout

Verification

  • rely on previous tests

Attribution

  • most of the work was done by @yeya24

@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch 4 times, most recently from ee4fbdc to 0d3ffa5 Compare October 4, 2023 18:25
@MichaHoffmann MichaHoffmann marked this pull request as draft October 4, 2023 18:31
yeya24
yeya24 reviewed Oct 4, 2023
View reviewed changes
pkg/query/querier.go Show resolved Hide resolved
pkg/query/querier.go Show resolved Hide resolved
pkg/rules/queryable.go Show resolved Hide resolved
@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch 4 times, most recently from 1d7896e to ceb48d4 Compare October 5, 2023 13:18
@MichaHoffmann MichaHoffmann marked this pull request as ready for review October 5, 2023 13:19
@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch 5 times, most recently from 1c9729b to a17bfd4 Compare October 5, 2023 13:27
@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch 4 times, most recently from 9bfebe0 to bd43490 Compare October 5, 2023 15:38
@yeya24
Copy link
Contributor

yeya24 commented Oct 5, 2023

Overall, this looks great! I retried the CI failure and we can merge if succeeds

@yeya24
Copy link
Contributor

yeya24 commented Oct 5, 2023 

=== NAME  TestStoreGateway
    store_gateway_test.go:117: store_gateway_test.go:117: ""
        
         unexpected error: block creation: add sample: 20 observations found in buckets, but the Count field is 9: histogram's observation count should be at least the number of observations found in the buckets

@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch from 96f7a46 to a1204f7 Compare October 5, 2023 17:49
@MichaHoffmann
Copy link
Contributor Author

MichaHoffmann commented Oct 5, 2023 •
edited 

=== NAME  TestStoreGateway
    store_gateway_test.go:117: store_gateway_test.go:117: ""
        
         unexpected error: block creation: add sample: 20 observations found in buckets, but the Count field is 9: histogram's observation count should be at least the number of observations found in the buckets

So the reason is that we try to write a malformed histogram and tsdb appends now have validation for that. I fixed it in the latest version of this PR.

@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch 2 times, most recently from b821d8e to 642e00e Compare October 6, 2023 06:52
fpetkovski
fpetkovski previously approved these changes Oct 6, 2023
View reviewed changes
pkg/block/indexheader/header_test.go
@@ -519,78 +521,3 @@ func readSymbols(bs index.ByteSlice, version, off int) ([]string, map[uint32]str
}
return symbolSlice, symbols, errors.Wrap(d.Err(), "read symbols")
}

func TestIndexHeaderV1LookupSymbols(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for the old TSDB 1 format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think yes, we added it recently but having Symbols as an interface just for this one test isnt much bang for the buck i figured.

@yeya24
Copy link
Contributor

yeya24 commented Oct 6, 2023

This seems reproducible every time.

=== NAME  TestStoreGatewayLazyExpandedPostingsEnabled
    store_gateway_test.go:1215: store_gateway_test.go:1215: ""
        
         unexpected error: unable to find metrics [thanos_bucket_store_lazy_expanded_postings_total] with expected values after 50 retries. Last error: <nil>. Last values: [0]

@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch 3 times, most recently from 73eb129 to 6b25638 Compare October 6, 2023 15:41
@yeya24 @MichaHoffmann
*: bump prometheus and promql-engine
08019c0 
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
@MichaHoffmann MichaHoffmann force-pushed the mhoffm-some-work-on-annotations-for-benye branch from 6b25638 to 08019c0 Compare October 6, 2023 17:10
@yeya24
Copy link
Contributor

yeya24 commented Oct 6, 2023

Nice work! Merging now

@yeya24 yeya24 enabled auto-merge (squash) October 6, 2023 17:40
@yeya24 yeya24 merged commit 56d8882 into thanos-io:main Oct 6, 2023
16 checks passed
coleenquadros pushed a commit to coleenquadros/thanos that referenced this pull request Oct 19, 2023
@MichaHoffmann @yeya24 @coleenquadros
*: bump prometheus and promql-engine (thanos-io#6772)
ace42d8 
Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
Co-authored-by: Ben Ye <benye@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yeya24 yeya24 yeya24 approved these changes

@fpetkovski fpetkovski fpetkovski left review comments

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@MichaHoffmann @yeya24 @fpetkovski