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: tighten up code around SLRU dir key handling #10082

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

jcsp
Copy link
Collaborator

@jcsp jcsp commented Dec 11, 2024

Problem

Changes in #9786 were functionally complete but missed some edges that made testing less robust than it should have been:

  • is_key_disposable didn't consider SLRU dir keys disposable
  • Timeline init_empty was always creating SLRU dir keys on all shards

The result was that when we had a bug (#10080), it wasn't apparent in tests, because one would only encounter the issue if running on a long-lived timeline with enough compaction to drop the initially created empty SLRU dir keys, and some CLog truncation going on.

Closes: https://github.com/neondatabase/cloud/issues/21516

Summary of changes

  • Update is_key_global and init_empty to handle SLRU dir keys properly -- the only functional impact is that we avoid writing some spurious keys in shards >0, but this makes testing much more robust.
  • Make test_clog_truncate explicitly use a sharded tenant

The net result is that if one reverts #10080, then tests fail (i.e. this PR is a reproducer for the issue)

@jcsp jcsp added c/storage/pageserver Component: storage: pageserver a/tech_debt Area: related to tech debt labels Dec 11, 2024
Copy link

github-actions bot commented Dec 11, 2024

6456 tests run: 6159 passed, 0 failed, 297 skipped (full report)


Flaky tests (4)

Postgres 17

Postgres 15

  • test_pgdata_import_smoke[None-1024-RelBlockSize.MULTIPLE_RELATION_SEGMENTS]: release-arm64

Postgres 14

Code coverage* (full report)

  • functions: 31.4% (8334 of 26538 functions)
  • lines: 47.7% (65626 of 137593 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
2624f2e at 2024-12-12T19:21:42.114Z :recycle:

@jcsp jcsp marked this pull request as ready for review December 12, 2024 18:17
@jcsp jcsp requested a review from a team as a code owner December 12, 2024 18:17
@jcsp jcsp requested a review from arssher December 12, 2024 18:17
@jcsp jcsp added this pull request to the merge queue Dec 16, 2024
Merged via the queue into main with commit ebcbc1a Dec 16, 2024
92 checks passed
@jcsp jcsp deleted the jcsp/fix-ingest-truncate-followup branch December 16, 2024 10:08
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.

2 participants