pageserver: wait for lsn lease duration after transition into AttachedSingle #9024

yliang412 · 2024-09-17T14:22:13Z

Part of #7497, closes #8890.

Problem

Since leases are in-memory objects, we need to take special care of them after pageserver restarts and while doing a live migration. The approach we took for pageserver restart is to wait for at least lease duration before doing first GC. We want to do the same for live migration. Since we do not do any GC when a tenant is in AttachedStale or AttachedMulti mode, only the transition from AttachedMulti to AttachedSingle requires this treatment.

Summary of changes

Added lsn_lease_deadline field in GcBlock::reasons: the tenant is temporarily blocked from GC until we reach the deadline. This information does not persist to S3.
In GCBlock::start, skip the GC iteration if we are blocked by the lsn lease deadline.
In TenantManager::upsert_location, set the lsn_lease_deadline to Instant::now() + lsn_lease_length so the granted leases have a chance to be renewed before we run GC for the first time after transitioned from AttachedMulti to AttachedSingle.

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

…dSingle Signed-off-by: Yuchen Liang <yuchen@neon.tech>

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

pageserver/src/tenant/tasks.rs

pageserver/src/tenant/gc_block.rs

pageserver/src/tenant/mgr.rs

github-actions · 2024-09-17T15:56:53Z

4968 tests run: 4804 passed, 0 failed, 164 skipped (full report)

Flaky tests (5)

Postgres 17

test_ondemand_wal_download_in_replication_slot_funcs: release-x86-64

Postgres 16

test_slots_and_branching: release-arm64
test_scrubber_physical_gc[4]: release-arm64

Postgres 15

test_subscriber_restart: release-x86-64

Postgres 14

test_scrubber_physical_gc[4]: release-x86-64

Code coverage* (full report)

functions: 31.9% (7425 of 23298 functions)
lines: 49.9% (59745 of 119838 lines)

* collected from Rust tests only

_{The comment gets automatically updated with the latest test results
d1b2bb1 at 2024-09-19T15:58:13.456Z :recycle:}

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

pageserver/src/tenant/gc_block.rs

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

pageserver/src/tenant/gc_block.rs

problame · 2024-09-18T13:33:17Z

(Removing myself from this review)

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

koivunej

Thanks, I think this is looking good.

I am surprised how many gc sensitive tests we have ... but which were not sensitive to lsn lease initial wait before? I'll try to understand these more and possibly chat with you...

pageserver/src/tenant/tasks.rs

…fety

… AttachedSingle (#9024)" This reverts commit 1708743.

…dSingle (#9024) Part of #7497, closes #8890. ## Problem Since leases are in-memory objects, we need to take special care of them after pageserver restarts and while doing a live migration. The approach we took for pageserver restart is to wait for at least lease duration before doing first GC. We want to do the same for live migration. Since we do not do any GC when a tenant is in `AttachedStale` or `AttachedMulti` mode, only the transition from `AttachedMulti` to `AttachedSingle` requires this treatment. ## Summary of changes - Added `lsn_lease_deadline` field in `GcBlock::reasons`: the tenant is temporarily blocked from GC until we reach the deadline. This information does not persist to S3. - In `GCBlock::start`, skip the GC iteration if we are blocked by the lsn lease deadline. - In `TenantManager::upsert_location`, set the lsn_lease_deadline to `Instant::now() + lsn_lease_length` so the granted leases have a chance to be renewed before we run GC for the first time after transitioned from AttachedMulti to AttachedSingle. Signed-off-by: Yuchen Liang <yuchen@neon.tech> Co-authored-by: Joonas Koivunen <joonas@neon.tech>

Part of #7497, closes #8817. ## Problem See #8817. ## Summary of changes **compute_ctl** - Renew lsn lease as soon as `/configure` updates pageserver_connstr, use `state_changed` Condvar for synchronization. **pageserver** As mentioned in #8817 (comment), we still want some permanent error reported if a lease cannot be granted. By considering attachment mode and the added `lsn_lease_deadline` when processing lease requests, we can also bound the case of bad requests to a very short period after migration/restart. - Refactor #9024 and move `lsn_lease_deadline` to `AttachedTenantConf` so timeline can easily access it. - Have separate HTTP `init_lsn_lease` and libpq `renew_lsn_lease` API. - Always do LSN verification for the initial HTTP lease request. - LSN verification for the renewal is **still done** when tenants are not in `AttachedSingle` and we have pass the `lsn_lease_deadline`, which give plenty of time for compute to renew the lease. **neon_local** - add and call `timeline_init_lsn_lease` mgmt_api at static endpoint start. The initial lsn lease http request is sent when we run `cargo neon endpoint start <static endpoint>`. ## Testing - Extend `test_readonly_node_gc` to do pageserver restarts and migration. ## Future Work - The control plane should make the initial lease request through HTTP when creating a static endpoint. This is currently only done in `neon_local`. Signed-off-by: Yuchen Liang <yuchen@neon.tech>

yliang412 added 2 commits September 17, 2024 10:14

pageserver: wait for lsn lease duration after transition into Attache…

7e4dded

…dSingle Signed-off-by: Yuchen Liang <yuchen@neon.tech>

update comments

7743144

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

yliang412 commented Sep 17, 2024

View reviewed changes

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

yliang412 marked this pull request as ready for review September 17, 2024 14:31

yliang412 requested a review from a team as a code owner September 17, 2024 14:32

yliang412 requested review from problame and koivunej September 17, 2024 14:32

yliang412 added the c/storage/pageserver Component: storage: pageserver label Sep 17, 2024

koivunej changed the title ~~pageserver: wait for lsn lease duration after transition into AttachedSignle~~ pageserver: wait for lsn lease duration after transition into AttachedSingle Sep 17, 2024

koivunej reviewed Sep 17, 2024

View reviewed changes

pageserver/src/tenant/gc_block.rs Show resolved Hide resolved

koivunej reviewed Sep 17, 2024

View reviewed changes

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

koivunej reviewed Sep 17, 2024

View reviewed changes

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

koivunej reviewed Sep 17, 2024

View reviewed changes

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

yliang412 added 4 commits September 17, 2024 12:28

refactor GCBlock reason storage

9c16c74

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

use reasons to block gc

af5e9b5

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

fix comments

8bcaa47

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

fix build

3b972d6

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

koivunej reviewed Sep 17, 2024

View reviewed changes

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

koivunej reviewed Sep 17, 2024

View reviewed changes

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

yliang412 added 2 commits September 17, 2024 16:32

drop pub; rename tenant_blocked field to be more informative

526d8a0

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

fix test_gc_blocking_by_timeline and test_sharding_split_compaction

b900db1

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

koivunej reviewed Sep 17, 2024

View reviewed changes

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

koivunej reviewed Sep 17, 2024

View reviewed changes

pageserver/src/tenant/gc_block.rs Show resolved Hide resolved

problame removed their request for review September 18, 2024 13:33

yliang412 added 4 commits September 18, 2024 11:08

use blocked until deadline mechanism

7d8bbdd

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

skip current round of gc if deadline not reached

9ad7033

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

set lsn_lease_length=0s in tests

78a1493

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

add lsn_lease_length=0s to more tests

7ef2421

Signed-off-by: Yuchen Liang <yuchen@neon.tech>

koivunej approved these changes Sep 19, 2024

View reviewed changes

koivunej reviewed Sep 19, 2024

View reviewed changes

pageserver/src/tenant/tasks.rs Show resolved Hide resolved

koivunej reviewed Sep 19, 2024

View reviewed changes

pageserver/src/tenant/tasks.rs Show resolved Hide resolved

koivunej added 4 commits September 19, 2024 09:33

test: remove extra tenant

a76e1f7

add a comment on what is the blocking

03873f5

doc

9ad42bf

doc: more stuff

6765254

yliang412 mentioned this pull request Sep 19, 2024

pageserver: provide more context in GCResult and clarify timeline_gc caller intent. #9063

Open

Merge branch 'main' into yuchen/lsn-lease-attached-multi-to-single-sa…

d1b2bb1

…fety

yliang412 enabled auto-merge (squash) September 19, 2024 15:05

yliang412 merged commit 1708743 into main Sep 19, 2024
78 checks passed

yliang412 deleted the yuchen/lsn-lease-attached-multi-to-single-safety branch September 19, 2024 16:27

yliang412 added a commit that referenced this pull request Sep 20, 2024

Revert "pageserver: wait for lsn lease duration after transition into…

d45ea92

… AttachedSingle (#9024)" This reverts commit 1708743.

yliang412 mentioned this pull request Sep 20, 2024

fix(pageserver): LSN lease edge cases around restarts/migrations #9055

Merged

5 tasks

yliang412 mentioned this pull request Sep 23, 2024

Epic: pageserver: LSN "leases" to block GC temporarily #7497

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pageserver: wait for lsn lease duration after transition into AttachedSingle #9024

pageserver: wait for lsn lease duration after transition into AttachedSingle #9024

yliang412 commented Sep 17, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024 •

edited

Loading

Postgres 17

Postgres 16

Postgres 15

Postgres 14

problame commented Sep 18, 2024

koivunej left a comment

pageserver: wait for lsn lease duration after transition into AttachedSingle #9024

pageserver: wait for lsn lease duration after transition into AttachedSingle #9024

Conversation

yliang412 commented Sep 17, 2024 • edited Loading

Problem

Summary of changes

Checklist before requesting a review

Checklist before merging

github-actions bot commented Sep 17, 2024 • edited Loading

4968 tests run: 4804 passed, 0 failed, 164 skipped (full report)

Postgres 17

Postgres 16

Postgres 15

Postgres 14

Code coverage* (full report)

problame commented Sep 18, 2024

koivunej left a comment

Choose a reason for hiding this comment

yliang412 commented Sep 17, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024 •

edited

Loading