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: option to run with just one tokio runtime #7331

Merged
merged 12 commits into from
Apr 8, 2024

Conversation

problame
Copy link
Contributor

@problame problame commented Apr 5, 2024

This PR is an off-by-default revision v2 of the (since-reverted) PR #6555 / commit 3220f830b7fbb785d6db8a93775f46314f10a99b.

See that PR for details on why running with a single runtime is desirable and why we should be ready.

We reverted #6555 because it showed regressions in prodlike cloudbench, see the revert commit message ad072de4209193fd21314cf7f03f14df4fa55eb1 for more context.

This PR makes it an opt-in choice via an env var.

The default is to use the 4 separate runtimes that we have today, there shouldn't be any performance change.

I tested manually that the env var & added metric works.

# undefined env var => no change to before this PR, uses 4 runtimes
./target/debug/neon_local start
# defining the env var enables one-runtime mode, value defines that one runtime's configuration
NEON_PAGESERVER_USE_ONE_RUNTIME=current_thread ./target/debug/neon_local start
NEON_PAGESERVER_USE_ONE_RUNTIME=multi_thread:1 ./target/debug/neon_local start
NEON_PAGESERVER_USE_ONE_RUNTIME=multi_thread:2 ./target/debug/neon_local start
NEON_PAGESERVER_USE_ONE_RUNTIME=multi_thread:default ./target/debug/neon_local start

I want to use this change to do more manualy testing and potentially testing in staging.

Future Work

Testing / deployment ergonomics would be better if this were a variable in pageserver.toml.
It can be done, but, I don't need it right now, so let's stick with the env var.

Copy link

github-actions bot commented Apr 5, 2024

2754 tests run: 2630 passed, 0 failed, 124 skipped (full report)


Code coverage* (full report)

  • functions: 28.0% (6412 of 22873 functions)
  • lines: 46.8% (45113 of 96303 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
aa5439c at 2024-04-08T13:17:09.005Z :recycle:

problame added a commit that referenced this pull request Apr 8, 2024
)

It's just unnecessary to use spawn_blocking there, and with
#7331 , it will result in
really just one executor thread when enabling one-runtime with
current_thread executor.
@problame problame requested a review from VladLazar April 8, 2024 12:29
@problame problame marked this pull request as ready for review April 8, 2024 12:30
@problame problame requested a review from a team as a code owner April 8, 2024 12:30
@problame problame merged commit 1081a4d into main Apr 8, 2024
54 checks passed
@problame problame deleted the problame/configurable-one-runtime branch April 8, 2024 14:27
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