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

Enable streaming chunks from store-gateways to queriers by default #6646

Merged

Conversation

charleskorn
Copy link
Contributor

@charleskorn charleskorn commented Nov 14, 2023

What this PR does

Similar to #6174, this PR enables streaming chunks from store-gateways to queriers by default.

This feature has been enabled in production at Grafana Labs for quite some time now, and we've had no issues with it.

I've left the option to enable / disable this feature as experimental so that we can remove it in 2.14 and always prefer streaming chunks.

Which issue(s) this PR fixes or relates to

(none)

Checklist

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

@charleskorn charleskorn marked this pull request as ready for review November 14, 2023 00:19
@charleskorn charleskorn requested review from a team as code owners November 14, 2023 00:19
@charleskorn charleskorn changed the title Enable streaming chunks from store-gateways to queriers by default. Enable streaming chunks from store-gateways to queriers by default Nov 14, 2023
@charleskorn
Copy link
Contributor Author

Putting back to draft while I investigate failures in the TestQuerierWithBlocksStorageRunningInSingleBinaryMode integration test.

@charleskorn charleskorn marked this pull request as draft November 14, 2023 03:26
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, glad to see this feature moving forward :)

The caching changes make sense to me. The only suggestions I left are to clarify the comments because it took me some time to get into this code again; maybe more verbose comments make this easier next time around

integration/querier_test.go Outdated Show resolved Hide resolved
integration/querier_test.go Outdated Show resolved Hide resolved
integration/querier_test.go Outdated Show resolved Hide resolved
integration/querier_test.go Outdated Show resolved Hide resolved
integration/querier_test.go Outdated Show resolved Hide resolved
@leizor
Copy link
Contributor

leizor commented Nov 28, 2023

The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase main and eventually move the CHANGELOG entry added / updated in this PR to the top of the CHANGELOG document. Thanks!

# Conflicts:
#	cmd/mimir/help-all.txt.tmpl
#	docs/sources/mimir/configure/about-versioning.md
#	pkg/querier/querier.go
# Conflicts:
#	CHANGELOG.md
…st with store-gateway chunks streaming enabled.
# Conflicts:
#	integration/querier_test.go
@charleskorn charleskorn force-pushed the charleskorn/store-gateway-chunks-streaming-by-default branch from 95ae131 to 91646ed Compare May 28, 2024 11:34
@charleskorn charleskorn marked this pull request as ready for review June 17, 2024 09:54
@charleskorn
Copy link
Contributor Author

#8039 has been running in production at Grafana for a few weeks now without any issues, so I think this is good to go now 🎉

Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

awesome, LGTM!

@charleskorn charleskorn enabled auto-merge (squash) June 17, 2024 22:51
@charleskorn charleskorn merged commit fee8a90 into main Jun 17, 2024
29 checks passed
@charleskorn charleskorn deleted the charleskorn/store-gateway-chunks-streaming-by-default branch June 17, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants