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

pageserver: cancellation for remote ops in tenant deletion on shutdown #6105

Merged
merged 5 commits into from
Mar 15, 2024

Conversation

jcsp
Copy link
Contributor

@jcsp jcsp commented Dec 11, 2023

Problem

Tenant deletion had a couple of TODOs where we weren't using proper cancellation tokens that would have aborted the deletions during process shutdown.

Summary of changes

  • Refactor enough that deletion/shutdown code has access to the TenantManager's cancellation toke
  • Use that cancellation token in tenant deletion instead of dummy tokens.

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

@jcsp jcsp added c/storage/pageserver Component: storage: pageserver a/tech_debt Area: related to tech debt labels Dec 11, 2023
Copy link

github-actions bot commented Dec 11, 2023

2712 tests run: 2586 passed, 1 failed, 125 skipped (full report)


Failures on Postgres 14

  • test_sharding_split_failures[failure10]: debug
# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_sharding_split_failures[debug-pg14-failure10]"
Flaky tests (2)

Postgres 14

  • test_pageserver_recovery: debug
  • test_empty_tenant_size: debug

Code coverage* (full report)

  • functions: 28.3% (7036 of 24824 functions)
  • lines: 47.0% (43467 of 92448 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
406317b at 2024-03-15T18:20:59.131Z :recycle:

@jcsp jcsp changed the title [wip] Jcsp/remote cancellation pt2 pageserver: cancellation for remote ops in tenant deletion on shutdown Dec 13, 2023
@jcsp jcsp force-pushed the jcsp/remote-cancellation-pt2 branch from 949a8dd to b591099 Compare December 15, 2023 18:08
jcsp added a commit that referenced this pull request Dec 22, 2023
## Problem

During startup, a tenant delete request might have to retry for many
minutes waiting for a tenant to enter Active state.

## Summary of changes

- Refactor delete_tenant into TenantManager: this is not a functional
change, but will avoid merge conflicts with
#6105 later
- Add 412 responses to the swagger definition of this endpoint.
- Use Tenant::wait_to_become_active in `TenantManager::delete_tenant`

---------

Co-authored-by: Arpad Müller <arpad-m@users.noreply.github.com>
abhigets pushed a commit that referenced this pull request Dec 22, 2023
## Problem

During startup, a tenant delete request might have to retry for many
minutes waiting for a tenant to enter Active state.

## Summary of changes

- Refactor delete_tenant into TenantManager: this is not a functional
change, but will avoid merge conflicts with
#6105 later
- Add 412 responses to the swagger definition of this endpoint.
- Use Tenant::wait_to_become_active in `TenantManager::delete_tenant`

---------

Co-authored-by: Arpad Müller <arpad-m@users.noreply.github.com>
@jcsp jcsp force-pushed the jcsp/remote-cancellation-pt2 branch from b591099 to bfa182c Compare January 9, 2024 12:44
@jcsp jcsp force-pushed the jcsp/remote-cancellation-pt2 branch from bfa182c to ec81f9a Compare March 15, 2024 09:52
@jcsp jcsp marked this pull request as ready for review March 15, 2024 09:53
@jcsp jcsp requested a review from a team as a code owner March 15, 2024 09:53
@jcsp jcsp requested a review from problame March 15, 2024 09:53
pageserver/src/task_mgr.rs Show resolved Hide resolved
pageserver/src/tenant/mgr.rs Outdated Show resolved Hide resolved
pageserver/src/tenant/delete.rs Show resolved Hide resolved
@jcsp jcsp enabled auto-merge (squash) March 15, 2024 16:25
@jcsp jcsp merged commit 1aa159a into main Mar 15, 2024
53 checks passed
@jcsp jcsp deleted the jcsp/remote-cancellation-pt2 branch March 15, 2024 18:03
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/pageserver Component: storage: pageserver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants