pageserver: generation number fetch on startup and use in /attach #5163

jcsp · 2023-08-31T13:29:53Z

Problem

Epic: split brain protection & multi-attach safety for pageserver #5050

Closes: #5136

Summary of changes

A new configuration property control_plane_api controls other functionality in this PR: if it is unset (default) then everything still works as it does today.
If control_plane_api is set, then on startup we call out to control plane /re-attach endpoint to discover our attachments and their generations. If an attachment is missing from the response we implicitly detach the tenant.
Calls to pageserver /attach API may include a generation parameter. If control_plane_api is set, then this parameter is mandatory.
RemoteTimelineClient's loading of index_part.json is generation-aware, and will try to load the index_part with the most recent generation <= its own generation.
The neon_local testing environment now includes a new binary attachment_service which implements the endpoints that the pageserver requires to operate. This is on by default if running cargo neon by hand. In test_runner/ tests, it is off by default: existing tests continue to run with in the legacy generation-less mode.

Caveats:

The re-attachment during startup assumes that we are only re-attaching tenants that have previously been attached, and not totally new tenants -- this relies on the control plane's attachment logic to keep retrying so that we should eventually see the attach API call. That's important because the /re-attach API doesn't tell us which timelines we should attach -- we still use local disk state for that. Ref: pageserver: enable arbitrary attachments during /re-attach #5173
Testing: generations are only enabled for one integration test right now (test_pageserver_restart), as a smoke test that all the machinery basically works. Writing fuller tests that stress tenant migration will come later, and involve extending our test fixtures to deal with multiple pageservers.
I'm not in love with "attachment_service" as a name for the neon_local component, but it's not very important because we can easily rename these test bits whenever we want.
Limited observability when in re-attach on startup: when I add generation validation for deletions in a later PR, I want to wrap up the control plane API calls in some small client class that will expose metrics for things like errors calling the control plane API, which will act as a strong red signal that something is not right.

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

github-actions · 2023-08-31T13:53:39Z

1640 tests run: 1567 passed, 0 failed, 73 skipped (full report)

Code coverage full report

functions: 52.6% (7530 of 14318 functions)
lines: 81.3% (44427 of 54629 lines)

_{The comment gets automatically updated with the latest test results
562e47e at 2023-09-06T13:41:27.664Z :recycle:}

When we refactor mgr.rs to implement a Mgr type, this arg count will go down.

jcsp · 2023-09-01T14:12:31Z

Please create a separate type and .rs file for the control_plane_api client (currently it's in mgr.rs).

Was going to do that later with the validation bits, but there's no time like the present :-) This is now control_plane_client.rs

Have you tried to introduce global parametrization for generation enabled/disabled, like we do with DEFAULT_PG_VERSION v14 and v15?

Right now what I care most about is not breaking the legacy mode (so still running all tests without generations). When it's time to check everything works in the new mode (i.e. before we think about cutting over in staging, then prod), it'll make sense to flip it back and forth globally.

…bers-pt2

control_plane/src/bin/attachment_service.rs

pageserver/src/tenant/mgr.rs

pageserver/src/tenant/remote_timeline_client.rs

control_plane/src/bin/attachment_service.rs

pageserver/src/control_plane_client.rs

pageserver/src/tenant/remote_timeline_client.rs

pageserver/src/tenant/remote_timeline_client/download.rs

Co-authored-by: Joonas Koivunen <[email protected]>

…bers-pt2

This was wrongly assuming generations should be the same: the local metadata will actually always have the current generation set, and in this situation we want to UseLocal if the sizes match, but take the metadata (generation) from the remote metadata.

koivunej

Few more nites nats, but this is looking good. Would like to focus on the filter_map vs. keeping the error, sounds like this should never happen and if there was a parsing error feels like we should not continue.

I don't see how that would be safe to continue.

control_plane/src/bin/attachment_service.rs

control_plane/src/bin/neon_local.rs

pageserver/src/tenant/mgr.rs

pageserver/src/tenant/remote_timeline_client.rs

pageserver/src/tenant/remote_timeline_client/download.rs

pageserver/src/tenant/timeline.rs

control_plane/src/attachment_service.rs

libs/remote_storage/src/local_fs.rs

pageserver/src/tenant/remote_timeline_client/download.rs

Co-authored-by: Joonas Koivunen <[email protected]>

## Problem Pageservers must not delete objects or advertise updates to remote_consistent_lsn without checking that they hold the latest generation for the tenant in question (see [the RFC]( https://github.com/neondatabase/neon/blob/main/docs/rfcs/025-generation-numbers.md)) In this PR: - A new "deletion queue" subsystem is introduced, through which deletions flow - `RemoteTimelineClient` is modified to send deletions through the deletion queue: - For GC & compaction, deletions flow through the full generation verifying process - For timeline deletions, deletions take a fast path that bypasses generation verification - The `last_uploaded_consistent_lsn` value in `UploadQueue` is replaced with a mechanism that maintains a "projected" lsn (equivalent to the previous property), and a "visible" LSN (which is the one that we may share with safekeepers). - Until `control_plane_api` is set, all deletions skip generation validation - Tests are introduced for the new functionality in `test_pageserver_generations.py` Once this lands, if a pageserver is configured with the `control_plane_api` configuration added in #5163, it becomes safe to attach a tenant to multiple pageservers concurrently. --------- Co-authored-by: Joonas Koivunen <[email protected]> Co-authored-by: Christian Schwarz <[email protected]>

libs: add Generation::previous

e839fd1

jcsp added 8 commits August 31, 2023 15:21

pageserver_api: add types for control plane calls

1cedf33

pageserver: optional generation input during create & attach

bf4572f

pageserver: add control_plane_api conf

233afac

pageserver: call into control plane on startup

ac8ef22

pageserver: if control plane API is disabled, ignore generations

44e6125

pageserver: generation-aware index_part.json loading

38d5dec

control_plane: add attachement_service, a mock control plane

885537a

neon_local: manage attachment_service

08fac67

jcsp force-pushed the jcsp/generation-numbers-pt2 branch from 4443dc9 to 6a952e4 Compare August 31, 2023 14:34

jcsp added 5 commits August 31, 2023 18:51

control_plane: implement attach hook

5e49aa5

remote_storage: fix LocalFs list_files

2ebe12e

tests: enable generations in neon_fixture

9deaa1b

clippy: permit many args to schedule_local_tenant_processing

af28cd1

When we refactor mgr.rs to implement a Mgr type, this arg count will go down.

Refactor TenantId encoding in JSON APIs

acce508

jcsp force-pushed the jcsp/generation-numbers-pt2 branch from 6a952e4 to acce508 Compare August 31, 2023 18:10

jcsp marked this pull request as ready for review August 31, 2023 18:12

jcsp requested review from a team as code owners August 31, 2023 18:12

jcsp requested review from lubennikovaav, problame and koivunej and removed request for a team and lubennikovaav August 31, 2023 18:12

jcsp added t/feature Issue type: feature, for new features or requests c/storage/pageserver Component: storage: pageserver labels Aug 31, 2023

clippy

1856d75

jcsp mentioned this pull request Sep 1, 2023

pageserver: enable arbitrary attachments during /re-attach #5173

Open

jcsp added 4 commits September 1, 2023 15:14

Fix python lint complaint

2553e48

Merge remote-tracking branch 'upstream/main' into jcsp/generation-num…

1cea6fb

…bers-pt2

fixup merge

2af98aa

Use safe tenant dir deletion in /re-attach handling

a88fa7d

koivunej reviewed Sep 5, 2023

View reviewed changes

jcsp and others added 12 commits September 6, 2023 09:22

Update pageserver/src/tenant/remote_timeline_client/download.rs

85c737e

Co-authored-by: Joonas Koivunen <[email protected]>

Apply suggestions from code review

03c5ab0

Co-authored-by: Joonas Koivunen <[email protected]>

Cleanup of attachment_service binary

382a930

improve logging in index part download

008545c

libs: flatten a match{}

864fb74

flatten a match{}

32548c7

Use DisplayFromStr instead of hex

bc32ec6

Add track_caller to functions that can panic

6467853

clippy

386542e

Add a log span to a unit test

8837479

Merge remote-tracking branch 'upstream/main' into jcsp/generation-num…

966b5e3

…bers-pt2

koivunej mentioned this pull request Sep 6, 2023

LSN and ids Serialize impls force unnecessary copy-paste #3511

Closed

koivunej reviewed Sep 6, 2023

View reviewed changes

control_plane/src/attachment_service.rs Outdated Show resolved Hide resolved

koivunej reviewed Sep 6, 2023

View reviewed changes

libs/remote_storage/src/local_fs.rs Show resolved Hide resolved

koivunej reviewed Sep 6, 2023

View reviewed changes

pageserver/src/tenant/remote_timeline_client/download.rs Outdated Show resolved Hide resolved

jcsp and others added 2 commits September 6, 2023 13:42

Apply suggestions from code review

db3dc06

Co-authored-by: Joonas Koivunen <[email protected]>

unwrap_or_else

562e47e

koivunej approved these changes Sep 6, 2023

View reviewed changes

jcsp merged commit 61d661a into main Sep 6, 2023

jcsp deleted the jcsp/generation-numbers-pt2 branch September 6, 2023 13:44

jcsp mentioned this pull request Sep 6, 2023

pageserver: deletion queue & generation validation for deletions #5207

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pageserver: generation number fetch on startup and use in /attach #5163

pageserver: generation number fetch on startup and use in /attach #5163

jcsp commented Aug 31, 2023 •

edited

Loading

github-actions bot commented Aug 31, 2023 •

edited

Loading

jcsp commented Sep 1, 2023

koivunej left a comment

pageserver: generation number fetch on startup and use in /attach #5163

pageserver: generation number fetch on startup and use in /attach #5163

Conversation

jcsp commented Aug 31, 2023 • edited Loading

Problem

Summary of changes

Checklist before requesting a review

Checklist before merging

github-actions bot commented Aug 31, 2023 • edited Loading

1640 tests run: 1567 passed, 0 failed, 73 skipped (full report)

Code coverage full report

jcsp commented Sep 1, 2023

koivunej left a comment

Choose a reason for hiding this comment

jcsp commented Aug 31, 2023 •

edited

Loading

github-actions bot commented Aug 31, 2023 •

edited

Loading