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

Update the spread-minimizing token migration documentation #8411

Merged
merged 4 commits into from
Jun 19, 2024

Conversation

duricanikolic
Copy link
Contributor

What this PR does

This PR enriches the existing "Migrate ingesters to spread-minimizing tokens" documentation with some missing parts. In particular:

  • It clarifies that it is necessary to disable ingester's read path shuffle-sharding before the migration, and specifies when it is possible to re-enable it once the migration is completed.
  • It states that it is necessary to delete the files where ingesters used to store the old tokens before the migration.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

  • [na] Tests updated.
  • Documentation added.
  • [na] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [na] about-versioning.md updated with experimental features.

@duricanikolic duricanikolic self-assigned this Jun 18, 2024
@duricanikolic duricanikolic requested review from jdbaldry and a team as code owners June 18, 2024 11:03
@duricanikolic duricanikolic added the type/docs Improvements or additions to documentation label Jun 18, 2024
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Copy link
Contributor

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM

@tacole02 tacole02 self-requested a review June 18, 2024 15:33
{{% /admonition %}}

{{% admonition type="note" %}}In order to prevent incorrect query results, [shuffle-sharding](https://grafana.com/docs/mimir/latest/configure/configure-shuffle-sharding/#ingesters-shuffle-sharding) on the [read path](https://grafana.com/docs/mimir/latest/configure/configure-shuffle-sharding/#ingesters-read-path) of your ingesters **must** be disabled before migrating ingesters to the spread-minimizing tokens. Shuffle-sharding on ingester's read path can be re-enabled at least `-querier.query-store-after` time after the last ingester zone was migrated to the spread-minimizing tokens.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unbold "must". We use bold text only for UI elements, not for emphasis.

{{% admonition type="note" %}}In order to prevent incorrect query results, [shuffle-sharding](https://grafana.com/docs/mimir/latest/configure/configure-shuffle-sharding/#ingesters-shuffle-sharding) on the [read path](https://grafana.com/docs/mimir/latest/configure/configure-shuffle-sharding/#ingesters-read-path) of your ingesters **must** be disabled before migrating ingesters to the spread-minimizing tokens. Shuffle-sharding on ingester's read path can be re-enabled at least `-querier.query-store-after` time after the last ingester zone was migrated to the spread-minimizing tokens.
{{% /admonition %}}

If ingesters are configured with a non-empty value of `-ingester.ring.tokens-file-path`, the latter represents the file where ingesters store the tokens at shutdown and restore them at startup. Keep track of this value, because it will be needed in the last step.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to "Keep track of this value, because you need it in the last step". This change avoids using "will" and also moves away from passive voice.

Copy link
Contributor Author

@duricanikolic duricanikolic Jun 19, 2024

Choose a reason for hiding this comment

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

Thank you. I've applied your suggestions.

If before the migration ingesters were configured to store their tokens under `-ingester.ring.tokens-file-path`, these files must be deleted once all ingester zones are migrated to the spread-minimizing tokens.

For example, assuming that an ingester pod `ingester-zone-a` from a namespace `mimir-prod` used to store its tokens in a file `/data/tokens`, the latter can be deleted by executing:

Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to rephrase this step to avoid passive tense. For example:

If, before the migration, you configured ingesters to store their tokens under -ingester.ring.tokens-file-path, you must delete these files after migrating all ingester zones to spread-minimizing tokens.

For example, if an ingester pod called ingester-zone-a from a namespace called mimir-prod used to store its tokens in a file called /data/tokens, you can run the following command to delete the /data/tokens file:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you

duricanikolic and others added 3 commits June 19, 2024 11:26
…s/index.md

Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
@duricanikolic duricanikolic merged commit 2ee427e into main Jun 19, 2024
29 checks passed
@duricanikolic duricanikolic deleted the yuri/update-smt-documentation branch June 19, 2024 11:44
@duricanikolic duricanikolic mentioned this pull request Jun 21, 2024
50 tasks
grafanabot pushed a commit that referenced this pull request Jun 21, 2024
* Update the spread-minimizing token migration documentation

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>

* Update docs/sources/mimir/configure/configure-spread-minimizing-tokens/index.md

Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>

* Fixing review findings

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>

---------

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>
(cherry picked from commit 2ee427e)
dimitarvdimitrov pushed a commit that referenced this pull request Jun 21, 2024
…8473)

* Update the spread-minimizing token migration documentation

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>

* Update docs/sources/mimir/configure/configure-spread-minimizing-tokens/index.md

Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>

* Fixing review findings

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>

---------

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>
(cherry picked from commit 2ee427e)

Co-authored-by: Đurica Yuri Nikolić <durica.nikolic@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-2.13 type/docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants