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: do big reads to fetch slru segment #11029

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

VladLazar
Copy link
Contributor

@VladLazar VladLazar commented Feb 27, 2025

Problem

Each page of the slru segment is fetched individually when it's loaded on demand.

Summary of Changes

Use Timeline::get_vectored to fetch 16 at a time.

TODO: does this stuff have any test coverage?

Problem

Each page of the slru segment is fetched individually when it's
loaded on demand.

Summary of Changes

Use `Timeline::get_vectored` to fetch 16 at a time.
Copy link
Contributor

@problame problame left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

let mut segment = BytesMut::with_capacity(n_blocks as usize * BLCKSZ as usize);
for blkno in 0..n_blocks {
let block = self
.get_slru_page_at_lsn(kind, segno, blkno, lsn, ctx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only remaining user of get_slru_page_at_lsn is map_all_timestamps, which is also invoked in a "for-each-block" loop.

Comment on lines +603 to +605
let keyspace = KeySpace::single(
slru_block_to_key(kind, segno, 0)..slru_block_to_key(kind, segno, n_blocks),
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

random note: we should probably use smallvec in KeySpace

slru_block_to_key(kind, segno, 0)..slru_block_to_key(kind, segno, n_blocks),
);

let partitions = keyspace.partition(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: name partition sounds like keyspace partitioning. batch, maybe?

Copy link

7744 tests run: 7366 passed, 0 failed, 378 skipped (full report)


Flaky tests (2)

Postgres 17

Code coverage* (full report)

  • functions: 32.8% (8644 of 26360 functions)
  • lines: 48.6% (73203 of 150497 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
f82edc6 at 2025-02-27T20:10:01.084Z :recycle:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants