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

Coalesce non-vectored get with vectored code (use vectored get for single key gets) #7381

Closed
4 tasks done
Tracked by #6296
jcsp opened this issue Apr 15, 2024 · 6 comments
Closed
4 tasks done
Tracked by #6296
Assignees
Labels
a/tech_debt Area: related to tech debt c/storage/pageserver Component: storage: pageserver

Comments

@jcsp
Copy link
Collaborator

jcsp commented Apr 15, 2024

Anticipate some performance work to make vectored get equally fast for single key case (e.g. #6932)

Ordered list of changes

  1. run-benchmarks
@jcsp jcsp added c/storage/pageserver Component: storage: pageserver a/tech_debt Area: related to tech debt labels Apr 15, 2024
@jcsp jcsp changed the title Coalesce non-vectored got with vectored code (use vectored get for single key gets) Coalesce non-vectored get with vectored code (use vectored get for single key gets) Apr 15, 2024
@jcsp
Copy link
Collaborator Author

jcsp commented Apr 22, 2024

Last week:

  • this shook out a bug that was fixed.
  • investigated why it's slower in some cases.

This week:

  • A fix for difference in how we handled aux files
  • Continue optimizing

@VladLazar
Copy link
Contributor

Last week:

  • Merged aux file handling fix
  • Subtle fix for materialized page cache usage
  • Optimised
  • Benchmarking - looks on par with legacy read path (even a bit better)
  • Ran with validation in staging over the weekend - no correctness issues

This week:

  • Run in staging without validation to qualify perf
  • Start staged roll out if above is ok

@VladLazar
Copy link
Contributor

Last week:

  • Qualified perf & correctness in staging/pre-prod

This week:

  • Deploy to ap-southeast regions & monitor

@VladLazar
Copy link
Contributor

Last week:

  • Deployed to ap-southeast-*. Small latency increased, but tput was much higher. CPU utilisation was better

This week:

  • Deploy to us-east-*

@koivunej
Copy link
Member

koivunej commented Jun 3, 2024

#7939 should be reverted when the default is changed.

koivunej added a commit that referenced this issue Jun 3, 2024
when running the regress tests locally without any environment variables
we use on CI, `test_pageserver_compaction_smoke` fails with division by
zero. fix it temporarily by allowing no vectored read happening. to be
cleaned when vectored get validation gets removed and the default value
can be changed.

Cc: #7381
@VladLazar
Copy link
Contributor

Rollout was completed a few weeks ago. At this point, the non-vectored read path is no longer in use (apart from validation which will be removed by #8005)

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

No branches or pull requests

3 participants