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: finish vectored get early #7490

Merged
merged 2 commits into from
Apr 24, 2024
Merged

Conversation

VladLazar
Copy link
Contributor

@VladLazar VladLazar commented Apr 23, 2024

Problem

If the previous step of the vectored left no further keyspace to investigate
(i.e. keyspace remains empty after removing keys completed in the previous step),
then we'd still grab the layers lock, potentially add an in-mem layer to the fringe and
at some further point read its index without reading any values from it.

Summary of changes

If there's nothing left in the current keyspace, then skip the search and just select
the next item from the fringe as usual.

When running test_pg_regress[release-pg16] with the vectored read path for singular
gets this improved perf drastically:

skip_vs_no_skip_pg_regress

Legend:
blue - without this patch
orange - with this patch

Correctness

Since no keys remained from the previous range (i.e. we are on a leaf node) there's nothing
that search can find in deeper nodes.

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

pageserver/src/tenant/timeline.rs Outdated Show resolved Hide resolved
pageserver/src/tenant/timeline.rs Show resolved Hide resolved
Copy link

github-actions bot commented Apr 23, 2024

2766 tests run: 2645 passed, 0 failed, 121 skipped (full report)


Code coverage* (full report)

  • functions: 28.1% (6479 of 23038 functions)
  • lines: 47.0% (46025 of 97941 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
e57853d at 2024-04-24T13:52:34.009Z :recycle:

Copy link
Member

@arpad-m arpad-m left a comment

Choose a reason for hiding this comment

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

Good catch!

@VladLazar VladLazar merged commit c12861c into main Apr 24, 2024
53 checks passed
@VladLazar VladLazar deleted the vlad/finish-vectored-get-early branch April 24, 2024 14:36
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.

3 participants