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

storcon: use attached shard counts for initial shard placement #8061

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

VladLazar
Copy link
Contributor

@VladLazar VladLazar commented Jun 14, 2024

Problem

When creating a new shard the storage controller schedules via Scheduler::schedule_shard. This does not take into account the number of attached shards. What it does take into account is the node affinity: when a shard is scheduled, all its nodes (primaries and secondaries) get their affinity incremented.

For two node clusters and shards with one secondary we have a pathological case where all primaries are scheduled on the same node. Now that we track the count of attached shards per node, this is trivial to fix. Still, the "proper" fix is to use the pageserver's utilization score.

Closes #8041

Summary of changes

Use attached shard count when deciding which node to schedule a fresh shard on.

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

  • Do not forget to reformat commit message to not include the above checklist

Copy link

github-actions bot commented Jun 14, 2024

3228 tests run: 3111 passed, 0 failed, 117 skipped (full report)


Code coverage* (full report)

  • functions: 32.3% (6840 of 21152 functions)
  • lines: 49.8% (53313 of 107107 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
c93b2b0 at 2024-06-20T16:15:05.567Z :recycle:

@VladLazar VladLazar force-pushed the vlad/storcon-new-tenant-placement branch from ab1aee5 to a5a2e98 Compare June 19, 2024 13:37
@VladLazar VladLazar requested a review from jcsp June 19, 2024 13:39
@VladLazar VladLazar marked this pull request as ready for review June 19, 2024 13:39
@VladLazar VladLazar requested a review from a team as a code owner June 19, 2024 13:39
@VladLazar VladLazar merged commit f8ac3b0 into main Jun 20, 2024
64 checks passed
@VladLazar VladLazar deleted the vlad/storcon-new-tenant-placement branch June 20, 2024 16:32
conradludgate pushed a commit that referenced this pull request Jun 27, 2024
## Problem
When creating a new shard the storage controller schedules via
Scheduler::schedule_shard. This does not take into account the number of
attached shards. What it does take into account is the node affinity:
when a shard is scheduled, all its nodes (primaries and secondaries) get
their affinity incremented.

For two node clusters and shards with one secondary we have a
pathological case where all primaries are scheduled on the same node.
Now that we track the count of attached shards per node, this is trivial
to fix. Still, the "proper" fix is to use the pageserver's utilization
score.

Closes #8041

## Summary of changes
Use attached shard count when deciding which node to schedule a fresh
shard on.
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.

storcon: initial placement of new tenants does not take into account number of attached shards
2 participants