pageserver: add `InProgress` tenant map state, use a sync lock for the map #5367

jcsp · 2023-09-25T15:08:51Z

Problem

Follows on from #5299

We didn't have a generic way to protect a tenant undergoing changes: Tenant had states, but for our arbitrary transitions between secondary/attached, we need a general way to say "reserve this tenant ID, and don't allow any other ops on it, but don't try and report it as being in any particular state".
The TenantsMap structure was behind an async RwLock, but it was never correct to hold it across await points: that would block any other changes for all tenants.

Summary of changes

Add the TenantSlot::InProgress value. This means:
- Incoming administrative operations on the tenant should retry later
- Anything trying to read the live state of the tenant (e.g. a page service reader) should retry later or block.
Store TenantsMap in std::sync::RwLock
Provide an extended get_active_tenant_with_timeout for page_service to use, which will wait on InProgress slots as well as non-active tenants.

Closes: #5378

pageserver/src/tenant/mgr.rs

github-actions · 2023-09-26T17:50:37Z

2358 tests run: 2241 passed, 0 failed, 117 skipped (full report)

Flaky tests (2)

Postgres 16

test_crafted_wal_end[last_wal_record_crossing_segment]: debug

Postgres 15

test_statvfs_pressure_min_avail_bytes: release

Code coverage (full report)

functions: 54.6% (8878 of 16272 functions)
lines: 81.6% (51148 of 62650 lines)

_{The comment gets automatically updated with the latest test results
b09891e at 2023-11-06T14:16:06.844Z :recycle:}

…ocations (#5299) ## Problem These changes are part of building seamless tenant migration, as described in the RFC: - #5029 ## Summary of changes - A new configuration type `LocationConf` supersedes `TenantConfOpt` for storing a tenant's configuration in the pageserver repo dir. It contains `TenantConfOpt`, as well as a new `mode` attribute that describes what kind of location this is (secondary, attached, attachment mode etc). It is written to a file called `config-v1` instead of `config` -- this prepares us for neatly making any other profound changes to the format of the file in future. Forward compat for existing pageserver code is achieved by writing out both old and new style files. Backward compat is achieved by checking for the old-style file if the new one isn't found. - The `TenantMap` type changes, to hold `TenantSlot` instead of just `Tenant`. The `Tenant` type continues to be used for attached tenants only. Tenants in other states (such as secondaries) are represented by a different variant of `TenantSlot`. - Where `Tenant` & `Timeline` used to hold an Arc<Mutex<TenantConfOpt>>, they now hold a reference to a AttachedTenantConf, which includes the extra information from LocationConf. This enables them to know the current attachment mode. - The attachment mode is used as an advisory input to decide whether to do compaction and GC (AttachedStale is meant to avoid doing uploads, AttachedMulti is meant to avoid doing deletions). - A new HTTP API is added at `PUT /tenants/<tenant_id>/location_config` to drive new location configuration. This provides a superset of the functionality of attach/detach/load/ignore: - Attaching a tenant is just configuring it in an attached state - Detaching a tenant is configuring it to a detached state - Loading a tenant is just the same as attaching it - Ignoring a tenant is the same as configuring it into Secondary with warm=false (i.e. retain the files on disk but do nothing else). Caveats: - AttachedMulti tenants don't do compaction in this PR, but they do in the follow on #5397 - Concurrent updates to the `location_config` API are not handled elegantly in this PR, a better mechanism is added in the follow on #5367 - Secondary mode is just a placeholder in this PR: the code to upload heatmaps and do downloads on secondary locations will be added in a later PR (but that shouldn't change any external interfaces) Closes: #5379 --------- Co-authored-by: Christian Schwarz <christian@neon.tech>

…chronous

down

Christian is out of office today

pageserver/src/deletion_queue/check.log

…tions-pt2

pageserver/src/http/routes.rs

pageserver/src/tenant/mgr.rs

jcsp · 2023-11-06T13:53:44Z

Test failure looks like a test helper issue:

pageserver/tests: "mocking error" panic in test_statvfs_pressure_min_avail_bytes #5794

koivunej · 2023-11-06T15:57:43Z

pageserver/src/tenant/mgr.rs

+unsafe impl Send for SlotGuard {}
+unsafe impl Sync for SlotGuard {}
+


No one questioned this?

thanks for pointing this out, cross linking #5802 which addresses this.

## Problem #5711 and #5367 raced -- the `SlotGuard` type needs `Gate` to properly enforce its invariant that we may not drop an `Arc<Tenant>` from a slot. ## Summary of changes Replace the TODO with the intended check of Gate.

jcsp mentioned this pull request Sep 26, 2023

pageserver: make TenantsMap lock synchronous, add explicit "busy" state to protect operations #5378

Closed

koivunej reviewed Sep 26, 2023

View reviewed changes

pageserver/src/tenant/mgr.rs Outdated Show resolved Hide resolved

jcsp force-pushed the jcsp/secondary-locations-pt2 branch from 4c6bd2e to 8307a31 Compare September 26, 2023 17:17

jcsp mentioned this pull request Sep 27, 2023

pageserver: location configuration API, attachment modes, secondary locations #5299

Merged

5 tasks

jcsp force-pushed the jcsp/secondary-locations-pt2 branch from 8307a31 to 99fd0d5 Compare September 29, 2023 14:47

jcsp force-pushed the jcsp/secondary-locations-pt2 branch 3 times, most recently from f995935 to c0f5248 Compare October 11, 2023 11:57

jcsp force-pushed the jcsp/secondary-locations-pt2 branch from c0f5248 to 071e06b Compare October 19, 2023 15:49

jcsp force-pushed the jcsp/secondary-locations-pt2 branch 3 times, most recently from a2dbce5 to d908b55 Compare October 31, 2023 09:46

jcsp self-assigned this Oct 31, 2023

jcsp force-pushed the jcsp/secondary-locations-pt2 branch from d908b55 to 4f29dfb Compare October 31, 2023 14:49

jcsp mentioned this pull request Oct 31, 2023

[WIP] Secondary locations heatmaps, downloads & tests #5745

Closed

5 tasks

jcsp force-pushed the jcsp/secondary-locations-pt2 branch from 4f29dfb to a2e7a79 Compare November 1, 2023 14:23

jcsp added 5 commits November 2, 2023 11:29

pageserver: add InProgress top level state & make TenantsMap lock syn…

7d75d1d

…chronous

pageserver: return 503 for GET on InProgress tenant

6671b77

pageserver: wait with timeout for InProgress slots in page service

cc2e062

tests: tolerate more diverse non-active tenant errors

a972ba1

pageserver: simplify GetActiveTenantError & add Cancelled case

19467ad

jcsp force-pushed the jcsp/secondary-locations-pt2 branch from a2e7a79 to 19467ad Compare November 2, 2023 11:31

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

jcsp marked this pull request as ready for review November 2, 2023 12:07

jcsp requested a review from a team as a code owner November 2, 2023 12:07

jcsp requested review from koivunej and removed request for a team November 2, 2023 12:07

jcsp added 7 commits November 3, 2023 10:36

Pull timeout_cancellable into utils

ce82103

pageserver: add pageserver_tenant_manager_ metrics

2f405ef

tests: check unexpected error counters at end of tests

cfaed2a

clippy

bbfef54

refactor match in tenant_map_acquire_slot_impl

1b119d3

Fix CLI tests for assert_no_log_errors

768da53

Make logic more explicit in SlotGuard::drop

130eadb

jcsp removed the request for review from koivunej November 3, 2023 15:38

jcsp added 2 commits November 3, 2023 16:04

Make SlotGuard stricter: refuse to clear a slot if old value isn't shut

a7a87b6

down

s/tenant_guard/slot_guard/

0cb75d5

shanyp approved these changes Nov 6, 2023

View reviewed changes

jcsp enabled auto-merge (squash) November 6, 2023 12:41

arpad-m reviewed Nov 6, 2023

View reviewed changes

pageserver/src/deletion_queue/check.log Outdated Show resolved Hide resolved

jcsp disabled auto-merge November 6, 2023 12:43

jcsp added 2 commits November 6, 2023 12:44

Remove a stray file

cefc99f

Merge remote-tracking branch 'upstream/main' into jcsp/secondary-loca…

f46cb2c

…tions-pt2

arpad-m requested changes Nov 6, 2023

View reviewed changes

pageserver/src/http/routes.rs Outdated Show resolved Hide resolved

pageserver/src/tenant/mgr.rs Outdated Show resolved Hide resolved

jcsp added 2 commits November 6, 2023 12:54

Refactor a match

a758ed6

De-indent function in a conditional

b09891e

jcsp requested a review from arpad-m November 6, 2023 12:56

arpad-m approved these changes Nov 6, 2023

View reviewed changes

jcsp enabled auto-merge (squash) November 6, 2023 13:04

This was referenced Nov 6, 2023

pageserver: use Gate for stronger safety check in SlotGuard #5793

Merged

pageserver: improve the shutdown log error #5792

Merged

jcsp merged commit 85cd97a into main Nov 6, 2023
38 checks passed

jcsp deleted the jcsp/secondary-locations-pt2 branch November 6, 2023 14:03

koivunej reviewed Nov 6, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pageserver: add `InProgress` tenant map state, use a sync lock for the map #5367

pageserver: add `InProgress` tenant map state, use a sync lock for the map #5367

jcsp commented Sep 25, 2023 •

edited

Loading

github-actions bot commented Sep 26, 2023 •

edited

Loading

Postgres 16

Postgres 15

jcsp commented Nov 6, 2023

koivunej Nov 6, 2023

arpad-m Nov 6, 2023

		unsafe impl Send for SlotGuard {}
		unsafe impl Sync for SlotGuard {}

pageserver: add InProgress tenant map state, use a sync lock for the map #5367

pageserver: add InProgress tenant map state, use a sync lock for the map #5367

Conversation

jcsp commented Sep 25, 2023 • edited Loading

Problem

Summary of changes

github-actions bot commented Sep 26, 2023 • edited Loading

2358 tests run: 2241 passed, 0 failed, 117 skipped (full report)

Postgres 16

Postgres 15

Code coverage (full report)

jcsp commented Nov 6, 2023

koivunej Nov 6, 2023

Choose a reason for hiding this comment

arpad-m Nov 6, 2023

Choose a reason for hiding this comment

pageserver: add `InProgress` tenant map state, use a sync lock for the map #5367

pageserver: add `InProgress` tenant map state, use a sync lock for the map #5367

jcsp commented Sep 25, 2023 •

edited

Loading

github-actions bot commented Sep 26, 2023 •

edited

Loading