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.

Sign up for GitHub

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

Jump to bottom

pageserver: clean up code for pre-LocationConf configuration #5388

Closed
Tracked by #5199
jcsp opened this issue Sep 26, 2023 · 1 comment · Fixed by #7947
Closed
Tracked by #5199

pageserver: clean up code for pre-LocationConf configuration #5388

jcsp opened this issue Sep 26, 2023 · 1 comment · Fixed by #7947
Assignees
@jcsp
Labels
a/tech_debt Area: related to tech debt c/storage/pageserver Component: storage: pageserver

Comments

@jcsp
Copy link
Contributor

jcsp commented Sep 26, 2023

This is tech debt.

In #5299, the new config-v1 tenant config file was added to hold the LocationConf type. We leave the old config file in place for forward compat.

One full deploy cycle later, we may remove the code for old style config.
@jcsp jcsp added c/storage/pageserver Component: storage: pageserver a/tech_debt Area: related to tech debt labels Sep 26, 2023
@jcsp jcsp self-assigned this Sep 26, 2023
@jcsp jcsp mentioned this issue Sep 26, 2023
Open
@jcsp
Copy link
Contributor Author

jcsp commented Feb 8, 2024

We should do this once the storage controller is prod ready, so that third parties have a good solution to get their generation numbers somewhere.

@jcsp jcsp mentioned this issue Feb 22, 2024
Merged
5 tasks
jcsp added a commit that referenced this issue Feb 25, 2024
@jcsp
pageserver: remove legacy attach/detach APIs from swagger (#6883)
8283779 
## Problem

Since the location config API was added, the attach and detach endpoints
are deprecated. Hiding them from consumers of the swagger definition is
a precursor to removing them

Neon's cloud no longer uses this api since
neondatabase/cloud#10538

Fully removing the APIs will implicitly make use of generation numbers
mandatory, and should happen alongside
#5388, which will happen once
we're happy that the storage controller is ready for prime time.

## Summary of changes

- Remove /attach and /detach from pageserver's swagger file
@jcsp jcsp mentioned this issue Jun 3, 2024
Merged
5 tasks
@jcsp jcsp closed this as completed in c39d5b0 Jun 26, 2024
conradludgate pushed a commit that referenced this issue Jun 27, 2024
@jcsp @conradludgate
pageserver: remove legacy tenant config code, clean up redundant gene…
d14cd6b 
…ration none/broken usages (#7947)

## Problem

In #5299, the new config-v1
tenant config file was added to hold the LocationConf type. We left the
old config file in place for forward compat, and because running without
generations (therefore without LocationConf) as still useful before the
storage controller was ready for prime-time.

Closes: #5388

## Summary of changes

- Remove code for reading and writing the legacy config file
- Remove Generation::Broken: it was unused.
- Treat missing config file on disk as an error loading a tenant, rather
than defaulting it. We can now remove LocationConf::default, and thereby
guarantee that we never construct a tenant with a None generation.
- Update some comments + add some assertions to clarify that
Generation::None is only used in layer metadata, not in the state of a
running tenant.
- Update docker compose test to create tenants with a generation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jcsp jcsp

Labels
a/tech_debt Area: related to tech debt c/storage/pageserver Component: storage: pageserver
Projects
None yet
Milestone
No milestone
1 participant
@jcsp