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 proper schedule context during node delete #9958

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

jcsp
Copy link
Collaborator

@jcsp jcsp commented Dec 1, 2024

Problem

I was touching test_storage_controller_node_deletion because for AZ scheduling work I was adding a change to the storage controller (kick secondaries during optimisation) that made a FIXME in this test defunct. While looking at it I also realized that we can easily fix the way node deletion currently doesn't use a proper ScheduleContext, using the iterator type recently added for that purpose.

Summary of changes

  • A testing-only behavior in storage controller where if a secondary location isn't yet ready during optimisation, it will be actively polled.
  • Remove workaround in test_storage_controller_node_deletion that previously was needed because optimisation would get stuck on cold secondaries.
  • Update node deletion code to use a TenantShardContextIterator and thereby a proper ScheduleContext

@jcsp jcsp added a/tech_debt Area: related to tech debt c/storage/controller Component: Storage Controller labels Dec 1, 2024
Copy link

github-actions bot commented Dec 1, 2024

7018 tests run: 6710 passed, 0 failed, 308 skipped (full report)


Flaky tests (4)

Postgres 17

Postgres 16

Code coverage* (full report)

  • functions: 30.4% (8275 of 27244 functions)
  • lines: 47.8% (65240 of 136588 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
9b58753 at 2024-12-02T13:15:48.023Z :recycle:

@jcsp jcsp marked this pull request as ready for review December 2, 2024 10:22
@jcsp jcsp requested a review from a team as a code owner December 2, 2024 10:22
@jcsp jcsp requested a review from problame December 2, 2024 10:22
@jcsp jcsp force-pushed the jcsp/storcon-delete-scheduling branch from 9d811dd to 2882af6 Compare December 2, 2024 10:23
@jcsp jcsp added this pull request to the merge queue Dec 3, 2024
Merged via the queue into main with commit aaee713 Dec 3, 2024
80 checks passed
@jcsp jcsp deleted the jcsp/storcon-delete-scheduling branch December 3, 2024 09:01
awarus pushed a commit that referenced this pull request Dec 5, 2024
## Problem

I was touching `test_storage_controller_node_deletion` because for AZ
scheduling work I was adding a change to the storage controller (kick
secondaries during optimisation) that made a FIXME in this test defunct.
While looking at it I also realized that we can easily fix the way node
deletion currently doesn't use a proper ScheduleContext, using the
iterator type recently added for that purpose.

## Summary of changes

- A testing-only behavior in storage controller where if a secondary
location isn't yet ready during optimisation, it will be actively
polled.
- Remove workaround in `test_storage_controller_node_deletion` that
previously was needed because optimisation would get stuck on cold
secondaries.
- Update node deletion code to use a `TenantShardContextIterator` and
thereby a proper ScheduleContext
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a/tech_debt Area: related to tech debt c/storage/controller Component: Storage Controller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants