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

Prefix 'out-of-order-blocks-external-label-enabled' with '.ingester' #4440

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

npazosmendez
Copy link
Contributor

What this PR does

We added this flag here #4182, but should have prefixed it with ingester..

Which issue(s) this PR fixes or relates to

n/a

Checklist

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

@npazosmendez npazosmendez changed the title Prefix 'ingester.' to 'out-of-order-blocks-external-label-enabled' Prefix 'out-of-order-blocks-external-label-enabled' with '.ingester' Mar 9, 2023
@npazosmendez npazosmendez marked this pull request as ready for review March 9, 2023 13:05
@npazosmendez npazosmendez requested a review from a team as a code owner March 9, 2023 13:05
CHANGELOG.md Outdated
@@ -72,7 +72,7 @@ Querying with using `{__mimir_storage__="ephemeral"}` selector no longer works.
* [FEATURE] Distributor, ingester: ingestion of native histograms. The new per-tenant limit `-ingester.native-histograms-ingestion-enabled` controls whether native histograms are stored or ignored. #4159
* [FEATURE] Query-frontend: Introduce experimental `-query-frontend.query-sharding-target-series-per-shard` to allow query sharding to take into account cardinality of similar requests executed previously. This feature uses the same cache that's used for results caching. #4121 #4177 #4188 #4254
* [ENHANCEMENT] Go: update go to 1.20.1. #4266
* [ENHANCEMENT] Ingester: added `out_of_order_blocks_external_label_enabled` shipper option to label out-of-order blocks before shipping them to cloud storage. #4182 #4297
* [ENHANCEMENT] Ingester: added `out_of_order_blocks_external_label_enabled` shipper option to label out-of-order blocks before shipping them to cloud storage. #4182 #4297 #4440
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'm guessing this will make it to 2.7.0, but I don't know enough about the release process to know if editing the changelog here (2.7.0-rc.0) is the right thing to do.

Copy link
Member

@pstibrany pstibrany Mar 9, 2023

Choose a reason for hiding this comment

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

We should not extend changelog entries from existing releases. This change needs changelog entry in main / unreleased section now.

Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this will make it to 2.7.0,

Since this is an experimental feature, I don't think we need to cherry-pick it onto release-2.7 branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done here db9c497 (#4440)

Just curious: if one wanted to include a hotfix in 2.7.0, what's the right way to update the changelog? Add a new secion 2.7.0-rc.1 and then all the 2.7.0-rc.* entries get combined into 2.7.0 once the release is done?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's an experimental feature, so no need to cherry pick it in 2.7. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Add a new secion 2.7.0-rc.1 and then all the 2.7.0-rc.* entries get combined into 2.7.0 once the release is done?

Yes, once final release is done, we collapse all individual -rc sections.

@npazosmendez npazosmendez requested a review from a team as a code owner March 9, 2023 13:13
Copy link
Member

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

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

Looks good to me now after the changelog update 👍

Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks!

@pracucci pracucci enabled auto-merge (squash) March 9, 2023 14:27
@pracucci pracucci merged commit 2c46f06 into main Mar 9, 2023
@pracucci pracucci deleted the njpm/fix-ingester-flag branch March 9, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants