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: always detach before deleting #8082

Merged
merged 7 commits into from
Jun 21, 2024
Merged

Conversation

jcsp
Copy link
Collaborator

@jcsp jcsp commented Jun 17, 2024

In #7957 we enabled deletion without attachment, but retained the old-style deletion (return 202, delete in background) for attached tenants. In this PR, we remove the old-style deletion path, such that if the tenant delete API is invoked while a tenant is detached, it is simply detached before completing the deletion.

This intentionally doesn't rip out all the old deletion code: in case a deletion was in progress at time of upgrade, we keep around the code for finishing it for one release cycle. The rest of the code removal happens in #8091

Now that deletion will always be via the new path, the new path is also updated to use some retries around remote storage operations, to tripping up the control plane with 500s if S3 has an intermittent issue.

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 17, 2024

2904 tests run: 2787 passed, 0 failed, 117 skipped (full report)


Flaky tests (2)

Postgres 14

  • test_location_conf_churn[3]: debug
  • test_subscriber_restart: release

Code coverage* (full report)

  • functions: 32.5% (6850 of 21077 functions)
  • lines: 49.9% (53337 of 106810 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
3ce9b55 at 2024-06-21T11:13:56.792Z :recycle:

@jcsp jcsp force-pushed the jcsp/always-detach-before-delete branch from 468941b to 262f00c Compare June 18, 2024 12:01
@jcsp jcsp changed the title Jcsp/always detach before delete pageserver: always detach before deleting Jun 18, 2024
@jcsp jcsp added c/storage/pageserver Component: storage: pageserver a/tech_debt Area: related to tech debt labels Jun 18, 2024
@jcsp jcsp marked this pull request as ready for review June 19, 2024 08:05
@jcsp jcsp requested a review from a team as a code owner June 19, 2024 08:05
@jcsp jcsp requested review from petuhovskiy and problame June 19, 2024 08:05
@jcsp
Copy link
Collaborator Author

jcsp commented Jun 21, 2024

(checking in with control plane folks that we're safe to merge this https://neondb.slack.com/archives/C03438W3FLZ/p1718966249028299?thread_ts=1718618574.225409&cid=C03438W3FLZ)

Copy link
Contributor

@problame problame left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

@jcsp jcsp merged commit 15728be into main Jun 21, 2024
62 of 64 checks passed
@jcsp jcsp deleted the jcsp/always-detach-before-delete branch June 21, 2024 14:39
jcsp added a commit that referenced this pull request Jun 24, 2024
…8091)

#8082 removed the legacy deletion path, but retained code for completing
deletions that were started before a pageserver restart. This PR cleans
up that remaining code, and removes all the pageserver code that dealt
with tenant deletion markers and resuming tenant deletions.

The release at #8138 contains
#8082, so we can now merge this
to `main`
conradludgate pushed a commit that referenced this pull request Jun 27, 2024
In #7957 we enabled deletion without attachment, but retained the
old-style deletion (return 202, delete in background) for attached
tenants. In this PR, we remove the old-style deletion path, such that if
the tenant delete API is invoked while a tenant is detached, it is
simply detached before completing the deletion.

This intentionally doesn't rip out all the old deletion code: in case a
deletion was in progress at time of upgrade, we keep around the code for
finishing it for one release cycle. The rest of the code removal happens
in #8091

Now that deletion will always be via the new path, the new path is also
updated to use some retries around remote storage operations, to
tripping up the control plane with 500s if S3 has an intermittent issue.
conradludgate pushed a commit that referenced this pull request Jun 27, 2024
…8091)

#8082 removed the legacy deletion path, but retained code for completing
deletions that were started before a pageserver restart. This PR cleans
up that remaining code, and removes all the pageserver code that dealt
with tenant deletion markers and resuming tenant deletions.

The release at #8138 contains
#8082, so we can now merge this
to `main`
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.

3 participants